New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MBS-10910: Display renamed from/into on label overview #1613
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shows the latest previous name and the first next name only. Maybe it should show all of the other names by checking these linked labels and so on? Showing begin/end dates would be nice too.
IMO that's correct - only those were renamed to and from this label. Any others were renamed to and from the other labels. |
This relationship seems to be transitive, so it seems it would be correct to show all the other names too. In either case, dates apply. |
Wouldn't the dates be just a duplicate of the label start and end dates? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rebased and pushed some minor changes here to use only one load_subset
call -- not that it's common for both directions to exist, but might as well.
But looking at it again, I'm wondering if we can improve the wording slightly. Let me know what you think.
root/label/LabelIndex.js
Outdated
@@ -51,6 +59,20 @@ const LabelIndex = ({ | |||
entity={label} | |||
numberOfRevisions={numberOfRevisions} | |||
/> | |||
{renamedFrom.length ? ( | |||
<RelatedEntitiesDisplay title={l('Renamed from')}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one sounds slightly strange to me though without both parts, as in "renamed from x to y." I'd suggest "Previously named" or "Previously known as." Do you think either of those would work?
<RelatedEntitiesDisplay title={l('Renamed from')}> | |
<RelatedEntitiesDisplay title={l('Previously named')}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like "Previously known as" a bit more so using that.
This seems like a sensible way to follow the flow of labels that were renamed, and I see no negatives about showing them there. I saw artist legal name is treated as only ever one result (which seems weird to me, but that's a separate issue). Because a label could be listed as renamed from or to multiple others, I pass these as arrays. I expect in most cases these would be data errors, but having them be visible makes them more likely to get fixed anyway.
Thanks! There hasn't been any follow-up about the dates in almost a year so I think it's best to merge this, as it's an improvement; it can certainly be improved further later on if people ask for dates. |
* master: Fix AddRelease preview display MBS-11840: Convert Reorder Mediums edit to React (#2226) Fix margin of link action icons (#2232) MBS-3774: Support setting date period to ext link (#2219) MBS-11891: Use https when linking to jira (#2229) MBS-2421: Show entity icon on relationship links Add inverted icons MBS-10910: Display renamed from/to on label overview (#1613) MBS-11835: Convert Change Wikidoc edit to React (#2222) Release Group -> Release group on edit labels Bump Flow to 0.158.0 MBS-11834: Convert Add Release edit to React
* beta: Update translations from Transifex Update POT files using the production database Update translations from Transifex MBS-11927: Display selected invalid link type MBS-11929: Skip validation of existing links MBS-11931: "Discogs" appears under "get the music" Update POT files using the production database Update translations from Transifex MBS-11922: Show historical (now secondary) types for old Add Release edits (#2243) MBS-11924: Hide useless tabs / subscribers section from deleted profiles (#2242) MBS-11933: Validate the code parameter in /oauth2/token (#2244) Remove useless arrows on add_cover_art images (#2240) Allow Babel to transform the punycode module Show label diff when removing it and hide label/catno when fully empty Ignore date if it's '' rather than passing '' through Update POT files using the production database Revert accidental name change (done for testing only) Update translations from Transifex MBS-11915: Don't show area icon if there's already a flag icon (#2238) Mark more types as read-only in externalLinks.js Bring /etc/service/webservice down by default Reuse website.service for the sitemaps container Better default GIT_* DBDefs subs Remove consul-template Multiple relationship auto-selection & validation MBS-9902(II), MBS-11912: Support type combination MBS-9902(I): Restrict link type options Add Flow support to URLCleanup MBS-11838: Convert Edit Release edit to React (#2230) Update translations from Transifex Update POT files using the production database Refactor URLCleanup, eliminate redundant constants Fix AddRelease preview display Fix order of imports MBS-11888: Automatically select / disable ended in DateRangeFieldset Add basic tests for release and instrument guess case MBS-11840: Convert Reorder Mediums edit to React (#2226) MBS-11898: Type update wrongly modified old links (#2231) Fix self -> this in GuessCaseHandlerRelease Fix margin of link action icons (#2232) MBS-11897: Beta: Statistics timeline broken MBS-3774: Support setting date period to ext link (#2219) Actually pass an isPerson boolean rather than typeID MBS-11891: Use https when linking to jira (#2229) MBS-2421: Show entity icon on relationship links Add inverted icons MBS-10910: Display renamed from/to on label overview (#1613) FormRowSortNameWithGuessCase: fix title, class MBS-11890: Beta: Fix alias "guess sort name" for persons MBS-11835: Convert Change Wikidoc edit to React (#2222) Release Group -> Release group on edit labels Bump Flow to 0.158.0 Update POT files using the production database Update translations from Transifex MBS-11650: Add tag and rating statistics to profile page Remove unneeded exp. MBS-11622: Clean up Apple Music label URLs (#2086) MBS-11875: Don't consider Braille an unlikely script (#2224) MBS-11846: Display release artist on release group view (#2201) Removing useless empty {} MBS-11839: Convert Remove Relationship Attribute edit to React Fix text overflow caused by long URL (#2216) MBS-11823: Don't break tags and genres mid-word (#2214) MBS-11837: Convert Edit Release Label edit to React (#2212) MBS-11836: Convert Edit Barcodes edit to React (#2209) MBS-11834: Convert Add Release edit to React Make guess case files flow strict Add flow types to GuessCase/Main Convert GuessCase/Main to a class Add flow types to guess case handlers Clarify variable names in guess case handlers Fix eslint issues in guess case handlers Fix typo: asterix -> asterisk Convert guess case handlers to classes Replace file-loader with asset modules Change artist id in t/sql/coverart.sql Remove @babel/register from node-runner Remove the TemplateMacros tests Migrate to Webpack v5 Remove .js from manifest.js paths Remove copy-webpack-plugin Remove EnvironmentPlugin Remove 'url' imports Bump copy-webpack-plugin to 6.4.0 Bump webpack-node-externals to 3.0.0 Bump webpack-manifest-plugin to 2.2.0 Bump terser-webpack-plugin to 4.2.3 Bump less-loader to 7.3.0 Migrate less from v2 to v4 Bump imports-loader to 1.2.0 Bump file-loader to 6.2.0 Bump webpack-cli to 3.3.12 Bump Webpack to 4.46.0 Return focus to selector if type is not selected Fix adding relationship when link is not submitted Allow submitting form from empty URL input Fix operator precedence in areRelationshipTargetGroupsEqual Update POT files using the production database Fix string to make it properly translatable Update translations from Transifex Update xgettext-js to 3.0.0 Update babel and associated plugins MBS-11722: Don't preselect basic as language proficiency (#2145) MBS-9426: Allow removing usernames from locked list MBS-11828: Add admin page to check whether username is locked MBS-11689: Report for pseudo-releases marked as the original tracklist (#2174) MBS-11854: Recognize unicode hyphen in guess case (#2199) MBS-11680: Group editing URL relationships by external link (#2114) Update HACKING to create the SELENIUM DB directly (#2210) MBS-11832: Add missing NotFound params for artist_credit (#2191) MBS-11693: Give useful message when rejecting Musixmatch /album links (#2126) MBS-11848: Add report for releases with Amazon cover art without CAA cover (#2197) MBS-7859: Hide irrelevant recording rels from release view (#2194) Fix deprecated link type code + tests (#2211) Use the $DIRECTION_FORWARD constant in place of 1 MBS-11856: Remove reports for releases with cover art relationships (#2202) MBS-11862: Do not show deprecated relationship types with 0 uses (#2206) MBS-11825: Use same order for art types when editing vs adding (#2203) Remove reporter id from report message-id (#2208) MBS-2221: Add date docs to relationship editor (#2204) MBS-11861: improve loopParity classes for tablesorter (#2205) (Re-)Add InformationIcon to components.js MBS-11863: Allow DNB links for works MBS-11864: Some DNB links are wrongly marked as invalid Allow declaring my $edit twice MBS-2418: Show Edit URL edits in entity edit histories MBS-11267: Always show artwork info when adding/reordering (#1821) MBS-11798: Disallow Instagram internal links (#2179) Fix typo (seriess -> series) Use q/qq to avoid escaping quotes for readability Move eslint-plugin-fb-flow to devDependencies Add eslint-plugin-fb-flow; fixed use-indexed-access-type Bump Flow to 0.157.0 Bump Flow to 0.156.0 Import Input/Output without passing through gc and drop gc.mode Fix Perl::Critic space at end of line errors on tests Fix Perl::Critic useless interpolation errors on tests Fix eslint sort-keys MBS-11850: Make footer links more visible Make most of GuessCaseT read-only Add flow types to guess-case/modes Clarify variable names Fix sort-keys issues Make fixEnglishKeyNames directly part of runPostProcess Replace Object.assign with spread Remove useless type declarations MBS-11824: Don't require space after feat. in reports MBS-11833: Drop "f." from the featured artists reports Add flow types to guess-case/Output Simplify appendWordPreserveWhiteSpace Clarify variable names Use .push to append to array Improve some comments, remove some useless ones Remove unused output.dropLastWord Clarify variable name: _w -> wordList Remove seemingly unused _output variable Fix eslint issues in GuessCaseOutput Convert GuessCaseOutput to class Add flow types to guess-case/Input Avoid negative null check in capitalizeCurrentWord Drop unneeded and misleading comment Remove unused insertWordAtEnd, simplify updateCurrentWord Drop unneeded wordListLength variable Drop self-explanatory / wrong comments, update others Clarify getCursorPosition / setCursorPosition Clarify variable names in GuessCaseInput Fix eslint issues in GuessCaseInput Convert GuessCaseInput to class More self-explanatory variable names for guess case MBS-11805: Add basic flow types to guess-case/utils MBS-11802: Add mock LinkType for examples (#2181) MBS-11793: Don't fetch Wikipedia abstract when URL is ended (#2178) MBS-11806: Don't group relationships for different track sets (#2186) MBS-11796: Add Internet Archive logo for sidebar (#2176) MBS-11808: Don't show tags in lists where vote count < 1 (#2188) Reject adding entities on search for all types that do not support it Fix autocomplete2 editor search MBS-11811: Do not batch-change track data for collapsed media Rename guessCaseMediaNames (it guesses tracks too) MBS-11812: Fix missing whitespace before "New medium title" MBS-11810: disc title should be medium title MBS-11805: Add flow types to guess-case/flags const over var for flags.js Sort keys Typo: hypen -> hyphen MBS-11797: Lowercase "censored", "uncensored", "explicit" in ETI MBS-11788: Guess case: Lowercase "official" in ETI MBS-11391: Add a dedicated popover to edit URL (#2151) MBS-11760: Improve testing for tag autodeletion MBS-11760: Add missing delete_unused_tag triggers MBS-11733: Remove WikiaParoles from lyrics whitelist MBS-11732: Remove LYRICSnMUSIC from lyrics whitelist
Implement MBS-10910
This seems like a sensible way to follow the flow of labels that were renamed, and I see no negatives about showing them there.
I saw artist legal name is treated as only ever one result (which seems weird to me, but that's a separate issue). Because a label could be listed as renamed from or into multiple others, I pass these as arrays. I expect in most cases these would be data errors, but having them be visible makes them more likely to get fixed anyway.