Skip to content
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

Display and preserve annotation of imported entities #1107

Merged
merged 7 commits into from
Aug 6, 2024

Conversation

kellnerd
Copy link
Contributor

@kellnerd kellnerd commented Jul 17, 2024

Display annotations for all types of imported entities and preserve the annotation during the whole "Edit & Approve" process.
The corresponding changes for the "Approve" action are in metabrainz/bookbrainz-data-js#321.

Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, perfect 🚀

kellnerd added 3 commits July 22, 2024 15:50
Loading of an edition group import into the entity editor and
`/imports/edition-group` routes were not working or being used.
This requires a BBID which imports do not have (yet):
> error: select distinct "bookbrainz"."edition".* from "bookbrainz"."edition" where "master" = $1 and "bookbrainz"."edition"."edition_group_bbid" in ($2) - invalid input syntax for type uuid: "190"
You may have to drop existing views manually because of a PG limitation.
@kellnerd
Copy link
Contributor Author

I've found a couple more issues while I was testing edition group imports (entity type name casing strikes again). One of them is even directly related to annotations, so I thought I might add these commits to this PR as well.

Comment on lines 34 to +38
'editionGroupType',
'editions.defaultAlias',
'editions.disambiguation',
'editions.releaseEventSet.releaseEvents'
// TODO: Reenable edition rels once imports have BBIDs
// 'editions.defaultAlias',
// 'editions.disambiguation',
// 'editions.releaseEventSet.releaseEvents'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unexpected trailing comma (comma-dangle)

Sigh, one more example why this ESLint rule is stupid... it often forces you to pointlessly edit neighbouring lines. Can we just disable this rule or migrate to always using dangling commas?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have my blessing to disable the rule. I've never seen it as a relevant code styling rule.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I've dropped the rule in 2fc11a2. We might want to do the same across all repos, but I better won't do that now or I will find some more rules which should be changed or turned off 🤐

The rule to disallow trailing commas often forces you to pointlessly
touch neighboring lines when you add or remove items.

If we want to enforce consistent dangling commas, we should rather force
them to always be present. But since our whole codebase follows this
rule, it would be better to have a grace period to gradually add commas.
@kellnerd kellnerd force-pushed the import-annotation branch from 8bbf264 to 2fc11a2 Compare July 24, 2024 17:21
@MonkeyDo MonkeyDo merged commit 4ee6307 into metabrainz:import-entities Aug 6, 2024
4 checks passed
@kellnerd kellnerd deleted the import-annotation branch August 12, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants