MBS-10659: Always show entity credits in rel edits #1398
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yvanzo
approved these changes
Feb 24, 2020
yvanzo
reviewed
Feb 24, 2020
Comment on lines
+94
to
+95
| entity0_credit => Optional[Str], | ||
| entity1_credit => Optional[Str], |
There was a problem hiding this comment.
I’m not sure about usingOptional over Nullable here. In both cases, tests need to be updated.
There was a problem hiding this comment.
I think Optional makes more sense since those aren't nullable in the DB. (Nullable is just defined as Optional[Maybe[x]].)
If one or both entities on the relationship have a credit, we want to still display that credit in the edit relationship edit, even if the edit itself doesn't change the credit (because it is still relevant to know the relationship is under a specific credit). Currently, the edit will display the main artist name at all times except if the credits are changing.
reosarevok
force-pushed
the
MBS-10659
branch
from
d6a5517
to
f62fe99
Compare
mwiencek
approved these changes
Feb 26, 2020
yvanzo
added a commit
that referenced
this pull request
Feb 26, 2020
* master: Update POT files using the production database Update translations from Transifex Fix warning about artwork image suffix on homepage (#1394) MBS-10659: Always show entity credits in rel edits (#1398) MBS-10166: Add genre entity to the WS (#1066) Fix warning: uninitialized value in numeric gt (>) (#1381) MBS-10617 : Convert Add Work edit to React Fix warning: uninitialized value in numeric eq (==) MBS-10058: Convert historic/merge_releases to React Remove entity lists from components.js MBS-10155: Make ReleaseGroupList always group by type MBS-10058: Convert merge_release_groups to React MBS-10058: Convert merge_works to React MBS-10058: Convert merge_series to React MBS-10058: Convert merge_recordings to React MBS-10058: Convert merge_places to React MBS-10058: Convert merge_labels to React MBS-10058: Convert merge_instruments to React MBS-10058: Convert merge_events to React Remove now unused merge_common.tt MBS-10058: Convert merge_artists to React MBS-10058: Convert merge_areas to React Fix warning: uninitialized value in numeric ne (!=) MBS-10594: Fail add annotation edit if entity is gone (#1388) Fix blog post link in generated git tag message (#1393) Document newly required version 10.15 of Node.js (#1392) MBS-9654: Show expanded ACs on recording merges (#1260) MBS-10643: Add autoselect for artists for CPDL (#1385) Eslint auto-fixes for comma-spacing Eslint auto-fixes for comma-dangle Eslint: fix non-auto-fixable comma-dangle Disable no-unused-vars where they're needed MBS-10649: Show empty diff side if label deselected MBS-10593: Deal with empty country id/name Fix handling of tape object serialization Merge CircleCI jobs Remove --appendonly from test redis config Massively speed up TruncateTables.sql in test env Clone mb-solr with depth 1 Recurse submodules at $MB_SOLR_TAG Don't compile client JS in CircleCI Run Selenium tests on Jenkins Allow setting the webpack mode directly Install PG extensions in Dockerfile.tests Install perl modules in Dockerfile.tests Support running Selenium tests in Firefox Bump chromedriver to 80.0.3987.16 Unbreak ujson in sir virtualenv Update test image Update to MB SOLR v.3.1.2 MBS-9491: Add genre MBIDs to the WS Ignore local::lib files for docker/js tools too (#1382) Replace git-core virtual package with git (#1380) Fix warning: uninitialized value in subroutine entry Fix warning: uninitialized value in addition (+) Fix warning: uninitialized value in subroutine entry
yvanzo
added a commit
that referenced
this pull request
Mar 2, 2020
* beta: Update POT files using the current code Update translations from Transifex MBS-10515: Fix sort of release recordings in merge (#1399) Update POT files using the production database Update translations from Transifex Fix warning about artwork image suffix on homepage (#1394) MBS-10659: Always show entity credits in rel edits (#1398) MBS-10166: Add genre entity to the WS (#1066) Fix warning: uninitialized value in numeric gt (>) (#1381) MBS-10617 : Convert Add Work edit to React Fix warning: uninitialized value in numeric eq (==) MBS-10058: Convert historic/merge_releases to React Remove entity lists from components.js MBS-10155: Make ReleaseGroupList always group by type MBS-10058: Convert merge_release_groups to React MBS-10058: Convert merge_works to React MBS-10058: Convert merge_series to React MBS-10058: Convert merge_recordings to React MBS-10058: Convert merge_places to React MBS-10058: Convert merge_labels to React MBS-10058: Convert merge_instruments to React MBS-10058: Convert merge_events to React Remove now unused merge_common.tt MBS-10058: Convert merge_artists to React MBS-10058: Convert merge_areas to React Fix warning: uninitialized value in numeric ne (!=) MBS-10594: Fail add annotation edit if entity is gone (#1388) Fix blog post link in generated git tag message (#1393) Document newly required version 10.15 of Node.js (#1392) MBS-9654: Show expanded ACs on recording merges (#1260) MBS-10643: Add autoselect for artists for CPDL (#1385) Eslint auto-fixes for comma-spacing Eslint auto-fixes for comma-dangle Eslint: fix non-auto-fixable comma-dangle Disable no-unused-vars where they're needed MBS-10649: Show empty diff side if label deselected MBS-10593: Deal with empty country id/name Fix handling of tape object serialization Merge CircleCI jobs Remove --appendonly from test redis config Massively speed up TruncateTables.sql in test env Clone mb-solr with depth 1 Recurse submodules at $MB_SOLR_TAG Don't compile client JS in CircleCI Run Selenium tests on Jenkins Allow setting the webpack mode directly Install PG extensions in Dockerfile.tests Install perl modules in Dockerfile.tests Support running Selenium tests in Firefox Bump chromedriver to 80.0.3987.16 Unbreak ujson in sir virtualenv Update test image Update to MB SOLR v.3.1.2 MBS-9491: Add genre MBIDs to the WS Ignore local::lib files for docker/js tools too (#1382) Replace git-core virtual package with git (#1380) Fix warning: uninitialized value in subroutine entry Fix warning: uninitialized value in addition (+) Fix warning: uninitialized value in subroutine entry
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MBS-10659
If one or both entities on the relationship have a credit, we want to still display that credit in the edit relationship edit, even if the edit itself doesn't change the credit (because it is still relevant to know the relationship is under a specific credit). Currently, the edit will display the main artist name at all times except if the credits are changing.