-
Notifications
You must be signed in to change notification settings - Fork 438
Enhance suffix unit conversion deletion with dependency checks #1561
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
Open
hmhngx
wants to merge
319
commits into
OpenEnergyDashboard:development
Choose a base branch
from
hmhngx:deletion-update
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes them static with the extra ones removed. See code comments.
… ChartDataSelectComponent
Co-authored-by: Ngoc Pham (Vy) <[email protected]> Co-authored-by: Wei Li <[email protected]> Co-authored-by: Alngdi Mohsen <[email protected]>
- Added group dependency checking in error handling - Moved dependency checking to happen before confirmation modal
- Update error message formatting to use React elements matching conversion deletion style - Reorder dependency checks: conversions first (highest priority), then meters/groups - Add logic to limit default graphic unit messages when conversion errors exist - Implement proper modal navigation to return to main units page on error - Add cancel modal for error notifications with proper action handling - Use structured message elements with bold text, lists, and proper formatting - Follow mentor feedback to match PR OpenEnergyDashboard#1505 patterns and logic
- Add English translations for unit deletion warnings - Add French translations with proper translation markers - Add Spanish translations for all supported languages - Include keys for: - unit.delete.meter.default.lost - unit.delete.group.default.lost - unit.default.graphic.unit - unit.delete.restricted - Support comprehensive error messaging in all languages
- Implement database transaction for unit deletion with default graphic unit updates - Update meters: SET default_graphic_unit = NULL WHERE default_graphic_unit = unitId - Update groups: SET default_graphic_unit = NULL WHERE default_graphic_unit = unitId - Delete unit after updating dependent records - Follow same transaction pattern as conversion deletion - Ensure database consistency during unit deletion process - Update success message to reflect meter/group updates
- Remove unnecessary default graphic unit checks (caught by conversion checks) - Fix message formatting to match conversion deletion style - Add forceCancel to error modal - Remove unused groupDataByID import
- Remove unit.delete.meter.default.lost - Remove unit.delete.group.default.lost - Remove unit.default.graphic.unit - Keys no longer needed after removing default graphic unit checks
- Remove unnecessary transaction code for default graphic unit updates - Extract req.body.id to const unitId - Simplify to direct unit deletion only
…ponent - Change actionConfirmMessage prop from required to optional - Conditionally render ModalBody only when actionConfirmMessage is provided - Allows modals with forceCancel=true to work without confirmation message when needed
- Simplify message building by pushing directly to msgElements instead of using intermediate arrays - Fix spacing in conversion error messages (add space before unit name in quotes) - Improve meter error message: make it bold and use 'The following meters use' for clarity - Move error header to beginning of message array using splice for better structure - Add actionConfirmMessage to cancel modal to display error messages when deletion is blocked - Remove unnecessary conditional checks and simplify control flow
- Add 'unit.delete.meters.use' key in English, French, and Spanish - Supports clearer error message format: 'The following meters use [unit] as meter unit'
…onversion involving a suffix unit is deleted.
… a conversion involving a suffix unit.
- Add checkUnitDependencies service for validation - Fix async race condition in removeAdditionalConversionsAndUnits - Add bidirectional conversion support and depth limiting - Enhance simulation to account for suffix unit cascades
- Add getConversionsByUnitId method - Add deleteConversionAndRelatedSuffixes with dependency checks
- Add dependency checks before suffix unit cleanup - Handle bidirectional conversions safely - Add row-level locking for concurrent operations - Improve error messages with meter/group details
- Show affected meters/groups in deletion warnings - Improve suffix unit warning formatting - Add translation keys for dependency warnings
- Fix setHelpLayout import in CompareLineChartComponent - Remove duplicate refreshingReadings in appStateSlice
Member
|
Thanks to @hmhngx for this contribution. I have not looked into why but a lot of files are showing as changes and about a dozen have merge conflicts. It appears work already in development is showing as changes. This need to be resolved before this PR can be reviewed. We can discuss if you like. |
Contributor
Author
|
Thanks for the feedback. The branch includes commits from other work, which
is causing the conflicts. Maybe I can create a clean branch from the latest
main with only the changes for issue #1448. Then I can rebase and resolve
conflicts, then update the PR.
…On Thu, Dec 11, 2025 at 7:53 AM Steven Huss-Lederman < ***@***.***> wrote:
*huss* left a comment (OpenEnergyDashboard/OED#1561)
<#1561 (comment)>
Thanks to @hmhngx <https://github.com/hmhngx> for this contribution. I
have not looked into why but a lot of files are showing as changes and
about a dozen have merge conflicts. It appears work already in development
is showing as changes. This need to be resolved before this PR can be
reviewed. We can discuss if you like.
—
Reply to this email directly, view it on GitHub
<#1561 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A56XWWU2EYQAKSBQWSVAIGD4BFSNVAVCNFSM6AAAAACOWJOCJSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMNBRG44DMNRUGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Enhances suffix unit conversion deletion to properly cascade delete related units/conversions when no dependencies exist, with improved UI warnings and consistency with unit edit/delete routes. Builds on the deletion branch of PR #1470.
Key improvements:
removeAdditionalConversionsAndUnits()using Promise.allFixes #1448
Type of change
Checklist
Limitations
None. All edge cases from PR #1470 have been addressed including bidirectional conversions, nested suffix chains, concurrent deletions, and orphan prevention.