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

Always show saveBlocker dialog for unclaimed blockers #4703

Merged
merged 4 commits into from Apr 2, 2024

Conversation

melton-jason
Copy link
Contributor

@melton-jason melton-jason commented Mar 29, 2024

Fixes #4693
Fixes #4705

The issue from #4693 (comment) was that adding a Col Obj Attribute to an existing CollectionObject saved triggered a resource.change for every field on the CollectionObject.
This caused business rules to be triggered for every field.
There is a uniqueness rule for CollectionObject which states that uniqueIdentifier must be unique at a database level.
Thus the uniqueness rule was triggered (as there are other CollectionObjects with NULL uniqueIdentifer) and the saveBlocker was set. Since the field was not present on the CollectionObject form, no indication was made to the user.

Checklist

  • Self-review the PR after opening it to make sure the changes look good
    and self-explanatory (or properly documented)
  • Add automated tests
  • Add relevant issue to release milestone

Testing instructions

  1. Make sure a CollectionObject from has the collectionObjectAttribute relationship as a subview and does not have the uniqueIdentifier field
  2. Find an existing CollectionObject which does not have a Col Obj Attribute
  3. Add a Col Obj Attribute to the Collection Object and ensure a saveBlocker is not set
  4. In SchemaConfig, make uniqueIdentifier required on CollectionObject
  5. Repeat steps 2-3, but this time make sure a saveBlocker is set and a dialog appears saying uniqueIdentifier must be unique to the database
  6. Add uniqueIdentifier to the CollectionObject form (or use the AutoGenerated Form) and ensure the uniqueness rule functions correctly

Fixes #4693
If the field is not required and the value is null or undefined,
do not invalidate rule.
Otherwise if field is requried and value is null or undefined,
invalidate rule.
@melton-jason melton-jason added this to the 7.9.4 milestone Mar 29, 2024
@melton-jason melton-jason requested review from a team March 29, 2024 18:46
@melton-jason
Copy link
Contributor Author

@specify/ux-testing

When testing this, can #4705 be tested as well? You should not be able to recreate the issue on this branch

Copy link
Contributor

@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.

Testing instructions

  1. Make sure a CollectionObject from has the collectionObjectAttribute relationship as a subview and does not have the uniqueIdentifier field
  2. Find an existing CollectionObject which does not have a Col Obj Attribute
  3. Add a Col Obj Attribute to the Collection Object and ensure a saveBlocker is not set

Can confirm that this fixes #4693

  1. In SchemaConfig, make uniqueIdentifier required on CollectionObject
  2. Repeat steps 2-3, but this time make sure a saveBlocker is set and a dialog appears saying uniqueIdentifier must be unique to the database
image
  1. Add uniqueIdentifier to the CollectionObject form (or use the AutoGenerated Form) and ensure the uniqueness rule functions correctly
image

Looks good! 👍

@bronwyncombs bronwyncombs self-requested a review April 1, 2024 14:53
Copy link

@bronwyncombs bronwyncombs left a comment

Choose a reason for hiding this comment

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

  1. ✅ Make sure a CollectionObject from has the collectionObjectAttribute relationship as a subview and does not have the uniqueIdentifier field
  2. ✅ Find an existing CollectionObject which does not have a Col Obj Attribute
  3. ✅ Add a Col Obj Attribute to the Collection Object and ensure a saveBlocker is not set
  4. ✅ In SchemaConfig, make uniqueIdentifier required on CollectionObject
  5. ✅ Repeat steps 2-3, but this time make sure a saveBlocker is set and a dialog appears saying uniqueIdentifier must be unique to the database
Screenshot 2024-04-01 at 9 49 47 AM
  1. ✅ Add uniqueIdentifier to the CollectionObject form (or use the AutoGenerated Form) and ensure the uniqueness rule functions correctly
Screenshot 2024-04-01 at 9 50 42 AM

In testing for locality with #4705:

Screenshot 2024-04-01 at 9 52 18 AM

Save behaves as expected for locality records added after initial save.

Looks good!

@lexiclevenger lexiclevenger self-requested a review April 1, 2024 20:10
Copy link
Collaborator

@lexiclevenger lexiclevenger left a comment

Choose a reason for hiding this comment

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

Testing instructions

  • Make sure a CollectionObject from has the collectionObjectAttribute relationship as a subview and does not have the uniqueIdentifier field
  • Find an existing CollectionObject which does not have a Col Obj Attribute
  • Add a Col Obj Attribute to the Collection Object and ensure a saveBlocker is not set
  • In SchemaConfig, make uniqueIdentifier required on CollectionObject
  • Repeat steps 2-3, but this time make sure a saveBlocker is set and a dialog appears saying uniqueIdentifier must be unique to the database
  • Add uniqueIdentifier to the CollectionObject form (or use the AutoGenerated Form) and ensure the uniqueness rule functions correctly
Screenshot 2024-04-01 at 3 14 00 PM

This also fixed issue #4705.

Initially, the save was blocked, but I left the tab and returned to it after some time and it was no longer blocked. I don't know what I did, and I was not able to recreate this.

Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

ups, pressed "Request changes" accidentally

@melton-jason melton-jason merged commit 968fc20 into production Apr 2, 2024
9 checks passed
@melton-jason melton-jason deleted the issue-4693 branch April 2, 2024 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
6 participants