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

Refactored all single record actions #9045

Conversation

bosiraphael
Copy link
Contributor

Context

Refactored all single record actions so they can be defined by a config file.
This refactoring is made with the idea that later the actions will be stored in the database, so we needed a way to serialize them.
For each object we can define a config file, if an object has no config file, it falls back to the default config.
I introduced action hooks, which return:

  • shouldBeRegistered: boolean Whether the action should be registered.
  • onClick: () => void The code that will be executed when we click on an action
  • ConfirmationModal?: React.ReactNode (optional) The confirmation modal which will be displayed on click

This PR also closes #8973

Next steps

  • Refactor multiple records actions
  • Refactor no selection actions
  • Add tests

@bosiraphael bosiraphael linked an issue Dec 12, 2024 that may be closed by this pull request
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR implements a major refactoring of single record actions to use a configuration-based approach, preparing for future database storage of action definitions.

  • Added DefaultSingleRecordActionsConfigV1.ts and DefaultSingleRecordActionsConfigV2.ts defining standard actions (favorites, delete) with consistent properties like type, scope, and hooks
  • Added WorkflowSingleRecordActionsConfig.ts with 8 workflow-specific actions (activate, deactivate, test, etc.) following the same configuration pattern
  • Introduced new action hook pattern returning shouldBeRegistered, onClick, and optional ConfirmationModal through actionHookResult.ts
  • Removed imperative action registration in favor of declarative configs, deleting files like SingleRecordActionMenuEntrySetter.tsx and useManageFavoritesSingleRecordAction.ts
  • Added feature flag IS_PAGE_HEADER_V2_ENABLED in RecordShowPage.tsx to control new action menu vs legacy workflow headers

32 file(s) reviewed, 28 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@thomtrp thomtrp left a comment

Choose a reason for hiding this comment

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

Please add tests for your hooks. Otherwise someone else will be stuck with coverage soon :)

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

I haven't tested it functionnally but the code looks great, love the approach
Please put back some tests!

@bosiraphael bosiraphael merged commit 8ce6f6d into main Dec 16, 2024
19 checks passed
@bosiraphael bosiraphael deleted the 8973-refactor-recordshowpageworkflowheader-to-use-contextual-actions branch December 16, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor RecordShowPageWorkflowHeader to use contextual actions
3 participants