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

'Save' on Edit Properties mutates objects multiple times for a single property change #5616

Closed
2 of 7 tasks
ozyx opened this issue Aug 4, 2022 · 3 comments
Closed
2 of 7 tasks
Labels
performance impacts or improves performance severity:critical type:bug
Milestone

Comments

@ozyx
Copy link
Member

ozyx commented Aug 4, 2022

Summary

Whenever the 'Save' (OK) button is clicked on the Edit Properties dialog, a separate mutation event is fired for each editable object property even if only for a single property change. This has performance and persistence implications when using an external persistence such as CouchDB. This also means that editing a single object property will cause multiple revisions of the object to register in CouchDB.

I believe this is ultimately leading to a lot of bugs related to editing properties (#5398, #5533 , partially #5138).

The behavior was modified in #4947, here, to use multiple mutate() calls instead of a single save() so that observers could listen on specific property changes.

Expected vs Current Behavior

Current: On hitting 'Save' in the Edit Properties dialog, objects are mutated once for each editable property regardless of whether or not their values have changed.
Expected:

  • If changes are made to one or more properties, a single mutation for the whole object should be performed.
  • Mutation events for each of the modified properties should be fired off individually.

Steps to Reproduce

This is easiest to see while using CouchDB plugin.

  1. Open an object's 'Edit Properties' dialog
  2. Without changing anything, click 'OK' (Save)
  3. In the Network tab, see that multiple unnecessary PUTs have been fired off.
  4. Notice that with each PUT, the object in the request remains unchanged.

Environment

Version: 2.1.0-SNAPSHOT
Build Date: Thu Aug 04 2022 13:02:04 GMT-0700 (Pacific Daylight Time)
Revision: 36736eb8a041acd3893b3223cd8bc93a7d4299db
Branch: release/2.0.7

Impact Check List

  • Data loss or misrepresented data?
  • Regression? Did this used to work or has it always been broken?
  • Is there a workaround available?
  • Does this impact a critical component?
  • Is this just a visual bug with no functional impact?
  • Does this block the execution of e2e tests?
  • Does this have an impact on Performance?

Additional Information

@ozyx ozyx added the type:bug label Aug 4, 2022
@unlikelyzero unlikelyzero added performance impacts or improves performance severity:critical labels Aug 5, 2022
@akhenry
Copy link
Contributor

akhenry commented Aug 5, 2022

Good catch. There appear to be two issues here:

  • The FormsAPI is reporting that every object property has changed, even when it hasn't.
    • I don't know what the root cause is here. Needs some investigation.
  • Mutating each property separately is triggering a persistence event for each.
    • I think the correct way to handle this is to open a transaction before initiating the property updates. This isn't possible right now outside of edit mode, but there is a PR coming that will enable this.

@akhenry akhenry assigned ozyx and unassigned ozyx Aug 23, 2022
@ozyx ozyx changed the title 'Save' on Edit Properties mutates objects multiple times even if they haven't changed 'Save' on Edit Properties mutates objects multiple times for a single property change Aug 24, 2022
@ozyx
Copy link
Member Author

ozyx commented Aug 24, 2022

  • The FormsAPI is reporting that every object property has changed, even when it hasn't.

Edited this issue and split this one off into its own separate issue here: #5705

@khalidadil
Copy link
Contributor

Tested by editing object properties and clicking "OK". I'm currently seeing only 1 PUT request on save.

Verified Fixed in Testathon on 10/27/22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance impacts or improves performance severity:critical type:bug
Projects
None yet
Development

No branches or pull requests

4 participants