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

feat: Entity merging tool #333

Merged
merged 101 commits into from
May 25, 2020
Merged

feat: Entity merging tool #333

merged 101 commits into from
May 25, 2020

Conversation

MonkeyDo
Copy link
Contributor

Problem

Plenty of duplicates in the database, and no way to merge them…

Solution

This implements merging routes to allow users to add entities to a merge queue.
merge-4

Once submitted, the user is redirected to a merging page with dropdowns to select which options to keep.
If the value is the same for all selected entities, it shows that single option (dropdown disabled).
If corrections need to be made, the user needs to modify the entity after merging.
merge-5

Merge revisions are show in a special way, and have a merge icon in front to denote them:

merge-6

merge-7

Areas of Impact

  • new /merge/… routes
  • database schema changes (for revisions)
  • small modifications of the existing entity create/edit process to account for merges.
  • new merge page and merge field elements
  • refactoring of some common utilities, trying to reuse as much of the existing entity save mechanism as possible
  • uses new version of bookbrainz-data

@MonkeyDo MonkeyDo marked this pull request as ready for review February 4, 2020 18:10
@coveralls
Copy link

coveralls commented Feb 4, 2020

Coverage Status

Coverage increased (+10.5%) to 60.244% when pulling 8e8cdb2 on entity-merging into 3846eec on master.

@endurance21
Copy link
Contributor

@MonkeyDo
Is there something I can work upon, here?.

@MonkeyDo
Copy link
Contributor Author

@MonkeyDo
Is there something I can work upon, here?.

This one is a pretty complex PR to jump into like that, but there is something you can do to help: testing ! :D
I published this PR on test.bookbrainz.org (the database is separate from the main website, just for testing).
If you could try out the merging tool that would be very helpful; you can either find duplicates by searching randomly (there's currently no tool or page to show duplicates), or create new fake entities to merge.
In particular, any scenario that involves relationships and links to other entities is preferable (I tested simple merges quite extensively already, but who knows what possibility I haven't thought of…)
Anything that looks out of place, broken, complicated to understand or could be improved, please create a ticket for :)

@endurance21
Copy link
Contributor

@MonkeyDo
Okay. I will report as soon as I wrap up my work!.

Deleted some previous work
Consider reworking and squashing this commit
This allows us to have the same submisison structure for entity and entity merge components.
The controller for merging pages was almost identical to the entity-editor. Refactored to make entity-editor controller work for regular edits and merges
There's no other way I can come up ith to know whether a revision is a merge or a standard edit
& Use  ORM option to keep multiple EntityRevisions with the same ID
Remove whatever I tried with revisionParents
so we can retrieve them from the body of the /edit/handler request, which sends the rootState
on all 5 $entity$_revision, so that we can deduce which entities have been merged by the revision.
Otherwise, if for example we're merging two authors, but there are relationships with another author, three author_revision rows will be created, but onyl two of the entities are merged.
Need some features from LTS version 0.33.1 (Panel subcomponents)
MonkeyDo added 22 commits May 1, 2020 12:55
And empty string date parsing issue
Includes a few fixes for issues (eg react proptypes) that were throwing errors
Use ISO strings everywhere we can and {year month day} objects only for date field and validation.
Rename date utility isNullValue to isNullDate.
This got borked during a rebase I think
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants