Read the directory notification types from commons-ui - #956
Conversation
The enum was duplicated in gridstudy and gridexplore, and the shared modification permissions hook needed it in commons-ui. The case export type moved along with it, as it travels on the same directory channel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The case export notification is not a directory notification, so it stays in this app, next to the only code reading it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Florent MILLOT <florent.millot_externe@rte-france.com>
| UPDATE_DIRECTORY = 'UPDATE_DIRECTORY', | ||
| CASE_EXPORT_FINISHED = 'caseExportFinished', | ||
| } | ||
| const CASE_EXPORT_FINISHED = 'caseExportFinished'; |
There was a problem hiding this comment.
Preferably, we should keep a single enum for all notification types intended for gridexplore-app, just like in gridstudy-app, otherwise, it will become complicated to manage and understand
There was a problem hiding this comment.
The thing is this one is not really related to the other directory enumeration. So I extracted the shared notification in the lib and I've let this one there.
…ry-notification-types
|
The DirectoriesNotificationType changes are unrelated to the permission work ! |
| */ | ||
| import type { UUID } from 'node:crypto'; | ||
|
|
||
| export enum NotificationType { |
There was a problem hiding this comment.
I don't agree for me it's related to this work
2 participants
PR Summary
Requires gridsuite/commons-ui#1261, which holds the shared declaration.