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

fix: Error when deleting local changes. #13669

Merged
merged 14 commits into from
Oct 8, 2024
Merged

fix: Error when deleting local changes. #13669

merged 14 commits into from
Oct 8, 2024

Conversation

Konrad-Simso
Copy link
Contributor

Description

This PR includes the following changes:

  • Bugfix
  • Refactoring of a query to split into a mutation and a useQuery.
  • Moved a common function between 2 files to be in a utils folder.
  • Added a test for new utils function and update old tests.

The bugfix was related to the order of when certain components were rerendering. This has been fixed by changing what is invalidated in the cache during deletion and when to load certain components based on query results.

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)

@github-actions github-actions bot added area/data-modeling Area: Related to data models - e.g. create, edit, use data models. solution/studio/designer Issues related to the Altinn Studio Designer solution. labels Oct 1, 2024
@Konrad-Simso Konrad-Simso linked an issue Oct 1, 2024 that may be closed by this pull request
@Konrad-Simso Konrad-Simso changed the title Fix: Error when deleting local changes. fix: Error when deleting local changes. Oct 1, 2024
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.25%. Comparing base (710812b) to head (5aebb81).
Report is 34 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #13669   +/-   ##
=======================================
  Coverage   94.24%   94.25%           
=======================================
  Files        1531     1532    +1     
  Lines       20885    20906   +21     
  Branches     2521     2522    +1     
=======================================
+ Hits        19684    19705   +21     
  Misses        948      948           
  Partials      253      253           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@mlqn mlqn left a comment

Choose a reason for hiding this comment

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

Nice work! 🚀 I just have one suggestion regarding the validation of the selectedOption / modelPath

@Konrad-Simso
Copy link
Contributor Author

Nice work! 🚀 I just have one suggestion regarding the validation of the selectedOption / modelPath

Your suggestions makes the bug exist again.
Without filtering in both places, SchemaEditorWithToolbar.tsx and TopToolbar.tsx, then some component will rerender with the wrong modelPath selected. I've added the filter to both files and removed the Utils function in the last commit.

Is this an okay solution?

@mlqn
Copy link
Contributor

mlqn commented Oct 7, 2024

I don't think we need to duplicate the find in both components. Did you pass the existingSelectedOption to the TopToolbar when you tested?
selectedOption={existingSelectedOption}

@Konrad-Simso
Copy link
Contributor Author

Konrad-Simso commented Oct 7, 2024

I don't think we need to duplicate the find in both components. Did you pass the existingSelectedOption to the TopToolbar when you tested? selectedOption={existingSelectedOption}

i'll try it

Edit: It worked, commit on the way

Copy link
Contributor

@mlqn mlqn left a comment

Choose a reason for hiding this comment

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

Nice work! 🚀

Copy link
Contributor

@JamalAlabdullah JamalAlabdullah left a comment

Choose a reason for hiding this comment

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

Tested ok 👍

@JamalAlabdullah JamalAlabdullah merged commit d2b2fba into main Oct 8, 2024
17 checks passed
@JamalAlabdullah JamalAlabdullah deleted the 13494-fix-reset branch October 8, 2024 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/data-modeling Area: Related to data models - e.g. create, edit, use data models. frontend solution/studio/designer Issues related to the Altinn Studio Designer solution.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when deleting local changes
3 participants