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

Datamodel updates not saved when just moving a field and then generating models. #13022

Closed
nkylstad opened this issue Jun 25, 2024 · 2 comments · Fixed by #13117
Closed

Datamodel updates not saved when just moving a field and then generating models. #13022

nkylstad opened this issue Jun 25, 2024 · 2 comments · Fixed by #13117
Labels
kind/bug Used when there is a defect / something is not working as it should. status/triage

Comments

@nkylstad
Copy link
Member

nkylstad commented Jun 25, 2024

Description of the bug

See title. Making only changes to the order of fields saves as expected, but if those are the only changes, then clicking "Generer modeller" directly after changing order of fields causes the order to revert back to the old order.

Steps To Reproduce

  1. Navigate to datamodels page
  2. Move a field up or down, and DO NOT make any other changes.
  3. Click "Generer modeller" to generate models and wait for OK response
  4. Re-load the page to re-fetch the model
  5. See that the field has reverted to its old position
  6. Move the field up or down again, and DO NOT make any other changes.
  7. Re-load the page to re-fetch the model
  8. See that the field is in its new updated position
  9. Click "Generer modeller" and wait for OK response
  10. Re-load the page again
  11. See that the field is in its updated position still.
Screen.Recording.2024-06-25.at.11.07.20.mov

Additional Information

Updated data model is saved as expected when just changing the order of fields, the issue seems to be with "Generer modeller" directly after changing the field order. Making any other changes, or re-loading the page after changing order, seems to resolve the issue.

Looks like there may be some outdated state used when "Generer modeller" is clicked that is causing this? Re-load of model before generating models seems to do the trick. We should ensure that the updated data model is passed to the backend when generating models.

@nkylstad nkylstad added kind/bug Used when there is a defect / something is not working as it should. status/triage labels Jun 25, 2024
@wrt95 wrt95 moved this to 📈 Todo in Team Studio Jul 2, 2024
@Jondyr Jondyr self-assigned this Jul 8, 2024
@Jondyr Jondyr moved this from 📈 Todo to 👷 In Progress in Team Studio Jul 8, 2024
@Jondyr
Copy link
Member

Jondyr commented Jul 8, 2024

This is very likely due to TanStack Query not updating the query data because TanStack uses structural sharing to only update query data that has been changed. Since the JSON specification defines objects as an unordered collection, and the property order is saved as an object, changing the order of the properties will not update the query data.

Screen.Recording.2024-07-08.133815.mp4

I think a proper solution to this would be to change properties to a JSON array to follow JSON specifications to avoid problems later, e.g.

"properties": [
  { "name": "property1", "type": "string" },
  { "name": "property2", "type": "string" }
]

But since I am not sure how big this refactoring would be, I will submit a smaller PR that disables structural sharing for just this query and we could discuss how to solve the root issue.

@Jondyr Jondyr linked a pull request Jul 8, 2024 that will close this issue
4 tasks
@Jondyr Jondyr moved this from 👷 In Progress to 🔎 Review in Team Studio Jul 8, 2024
@Jondyr Jondyr removed their assignment Jul 8, 2024
@standeren standeren assigned standeren and unassigned standeren Jul 8, 2024
@github-project-automation github-project-automation bot moved this from 🔎 Review to 🧪 Test in Team Studio Jul 9, 2024
@Jondyr Jondyr removed their assignment Jul 9, 2024
@lassopicasso lassopicasso self-assigned this Jul 9, 2024
@lassopicasso
Copy link
Contributor

Tested in dev - OK :)
Nice solution and research. I agree with you that should have some discussion around this on a later stage.

@lassopicasso lassopicasso removed their assignment Jul 9, 2024
@lassopicasso lassopicasso moved this from 🧪 Test to ✅ Done in Team Studio Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Used when there is a defect / something is not working as it should. status/triage
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants