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

Agent merging #2579

Merged
merged 71 commits into from
May 19, 2023
Merged

Agent merging #2579

merged 71 commits into from
May 19, 2023

Conversation

maxpatiiuk
Copy link
Member

@maxpatiiuk maxpatiiuk commented Dec 10, 2022

Fixes #1030
Fixes #2692

@maxpatiiuk maxpatiiuk changed the base branch from production to testability December 13, 2022 23:06
@maxpatiiuk maxpatiiuk changed the base branch from testability to production December 13, 2022 23:06
@maxpatiiuk
Copy link
Member Author

Pretty much finished. Now just to polish it

Screenshot 2022-12-26 at 19 35 27

Screenshot 2022-12-26 at 19 35 35

@maxpatiiuk maxpatiiuk requested a review from a team December 27, 2022 01:42
@maxpatiiuk maxpatiiuk marked this pull request as ready for review December 27, 2022 01:42
@grantfitzsimmons
Copy link
Member

grantfitzsimmons commented Dec 27, 2022

After merging three agents, I get a 404 error above the query builder.

Screenshot 2022-12-27 at 6 37 45 AM

https://newmexico-agent-merging.test.specifysystems.org/specify/query/14/

Can be recreated by selecting a result after performing a merge

@grantfitzsimmons
Copy link
Member

image

After merging 3 agents, only one of the items disappears and one is left behind (outlined in red). When going to that record, it does not exist.

@grantfitzsimmons
Copy link
Member

Screen.Recording.2022-12-27.at.6.43.28.AM.mp4

Example workflow when merging a large batch of agents.

Frequent errors in the console reporting

{type: 'invalidResponseCode', statusText: Array(3), responseText: 'No Agent matches the given query.'}

Takes quite a while to complete the merging, and shows a dialog for a second that is confusing to the user after beginning the merge.

This DB is really good for testing this on (https://newmexico-agent-merging.test.specifysystems.org/specify/query/14/)

@grantfitzsimmons
Copy link
Member

After selecting 3 agents in a new tab (after closing all other tabs) and after all actions appeared to be finished, I get an out of date error.

image

Agent object 47591 is out of date

Specify 7 Crash Report - 2022-12-27T12_50_44.370Z.txt


Can be recreated by running the query and selecting the following 3 results:
image

Recieved this error upon recreation:

Agent object 47590 is out of date

@grantfitzsimmons
Copy link
Member

The issue where a new merge dialog appears while merging is taking place with the 404 error after the action is taken is something that needs to be resolved

@grantfitzsimmons
Copy link
Member

grantfitzsimmons commented Dec 27, 2022

Screen.Recording.2022-12-27.at.6.55.08.AM.mp4

Here's another example.

See that

  • Temporary merge dialog appears that differs from the initial one
  • 404 error page is shown above the query but the query remains functional
  • After merging there are 4 remaining records in the query results. Refreshing the page and running the query shows the proper count returned

https://newmexico-agent-merging.test.specifysystems.org/specify/query/15/

@maxpatiiuk
Copy link
Member Author

All of the above should be fixed now.
However, you might not able to test the fix until #2713 is fixed

@grantfitzsimmons
Copy link
Member

grantfitzsimmons commented Dec 27, 2022

Screen.Recording.2022-12-27.at.11.06.04.AM.mp4

Same second dialog problem happening after initiating the merge

https://newmexico-agent-merging.test.specifysystems.org/specify/query/16/

Copy link
Member

@grantfitzsimmons grantfitzsimmons left a comment

Choose a reason for hiding this comment

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

Same second dialog problem happening after initiating the merge

@maxpatiiuk
Copy link
Member Author

Same second dialog problem happening after initiating the merge

This is because an error has occurred (you see that you had 4 columns, but the final dialog has only 3 - one record was merged successfully, the next one failed)

The problem is that it doesn't show you the error messages - that was a bug introduced on this branch and fixed. The bug appears fixed for me (tried on coldfish on test server). Maybe the server just didn't update on time when you tested. Could you try this again?

@maxpatiiuk
Copy link
Member Author

I forgot to mention during the demo that agent merging is also available though the Form Meta menu.

Reasons:

  1. Allows people with no query builder access to use it
  2. It's going to be a well-advertised feature of Specify 7 - we need to include it in more than one place in the UI

@chanulee1
Copy link
Contributor

What needs to be tested on this branch?

@maxpatiiuk
Copy link
Member Author

Besides the logic for properly handling business rules, agent merging is now feature complete.

Context:
Agent merging allows to merge duplicate agent merging
In the future, we would enable this feature for tables other than Agent
You can access this feature by selecting multiple agents in the query results or by opening an agent form, opening Form Meta menu, and clicking on "Merge Record"

To test:
Interface for merging records
Interface for merging -to-many relationships (merging Agent Addresses or Agent members)
Pressing the "Merge" button would do merging record by record. If it fails, a dialog will be shown. The dialog should be dismissible. Once you resolve the business rule violation mentioned in the error dialog, the records should be mergable.
After merging, the duplicates should be removed from the query results. Similarly, if you opened "Record Merging" from the form, after merging all, it should open the final record in a form. If you were in a record set while doing that, the final record should also be opened in a context of a record set.

Also, I once had this error happen:
Screenshot 2023-01-13 at 09 27 32

Please tell if you also have it happen.

@maxpatiiuk
Copy link
Member Author

maxpatiiuk commented Jan 20, 2023

Agent Merging dialog is now more polished (based on the feedback I received). Changes:

  1. The references to this record section are cleaned up (now it skips showing tables like Loan Agent and goes directly to Loan, and instead of displaying each resource separately, displays a count - clicking on it shows a dialog like in "Browse in Forms")
  2. Timestamps are shown as relative dates
  3. Added a vertical line to the right of the final record to make it more distinct (also made the heading bold and renamed the button labels in the "Preview" row)
  4. Auto merge no longer runs when you open the dialog. Instead, you can run it on demand by pressing a button
  5. There is an "x" button in the heading of each resource that allows dismissing that resource from merging (if you found out that it's not actually a duplicate)

Screenshot 2023-01-19 at 21 58 50

@chanulee1
Copy link
Contributor

Errors when attempting to merge certain agents with any other agent record, such as User, Test C and Bentley, Andrew C. Errors occur while the 'References to this record' section is loading.

User, Test C:
image

Specify 7 Crash Report - 2023-01-20T18_54_19.275Z.txt

Bentley, Andrew C:
image

Specify 7 Crash Report - 2023-01-20T19_14_41.904Z.txt

@chanulee1
Copy link
Contributor

'References to this record' section overflows for some agent records.
image

@chanulee1
Copy link
Contributor

Is this expected behavior? Deleting one of the address records in the merged agent record also deletes the corresponding address record for a duplicate agent when the address merging interface is closed. Unable to get this record back without restarting the agent merging interface.

1EQ3GzrwHV.mp4

@chanulee1
Copy link
Contributor

Deleting multiple records in merged record indents field labels in -to-many relationship merging interface.

HVHgnFP7ju.mp4

@chanulee1
Copy link
Contributor

Merging two agent records with respective address records creates agent record with three address records (Two copies of records from Duplicate 2). Also, when no value is chosen for the 'first name' field of the merged record, the query results display the Duplicate 1's value for this field after merging. Going to the merged record shows that this value was not saved to this field.

rxzGfkBng6.mp4

Similar behavior when merging more than two agents.

qJB43RqirN.mp4

@maxpatiiuk maxpatiiuk changed the base branch from production to xml-editor May 19, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants