-
Notifications
You must be signed in to change notification settings - Fork 0
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
JNG-6072 dialog from row lifecycle #499
Conversation
noherczeg
commented
Dec 17, 2024
•
edited by github-actions
bot
Loading
edited by github-actions
bot
JNG-6072 Running operations in dialogs where dialogs have been opened from a table row cause UI to crash |
- if a certain operation effectively removed the row from the table from which we opened it
- the operation caused the root page to reload
- the reload removed the element from the table
- the dialog got notified that it should reload
- the reload caused the re-calculation to yield either a different instance (tried to get the same row index from a new set, holy shit) or undefined because there were no rows left
WalkthroughThe pull request introduces defensive data handling improvements in React components, focusing on preventing potential crashes during data updates. The changes add conditional checks to ensure that data is valid before updating state, specifically addressing scenarios where rows might be removed from source tables while still being referenced in dialogs or subscriptions. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
judo-ui-react/src/main/resources/actor/src/dialogs/index.tsx.hbs (1)
390-395
: LGTM! Good defensive programming practice.The added null check prevents crashes in edge cases where a row might be removed from the original table while being viewed in a dialog. This is a good defensive programming practice.
Consider adding error logging to track these edge cases for monitoring purposes.
if (copy !== undefined && copy !== null) { + console.debug('Skipping data update due to null/undefined value', { + dataPath, + rootPageId, + }); setData(copy); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
judo-ui-react-itest/ActionGroupTest/action_group_test__god/src/test/resources/snapshots/frontend-react/src/dialogs/View/Galaxy/Stars/RelationViewPage/index.tsx.snapshot
(1 hunks)judo-ui-react/src/main/resources/actor/src/dialogs/index.tsx.hbs
(1 hunks)
🔇 Additional comments (1)
judo-ui-react-itest/ActionGroupTest/action_group_test__god/src/test/resources/snapshots/frontend-react/src/dialogs/View/Galaxy/Stars/RelationViewPage/index.tsx.snapshot (1)
668-673
: LGTM! Changes are consistent with the template.
The defensive null check implementation matches the changes in the template file, ensuring consistent behavior across the codebase.