Skip to content

Add sortOrder property to configurable actions and implement sorting …#4348

Merged
IvanIlyichev merged 10 commits intoshesha-io:mainfrom
Tshepo1103-lab:tshepo/action-configurations
Dec 18, 2025
Merged

Add sortOrder property to configurable actions and implement sorting …#4348
IvanIlyichev merged 10 commits intoshesha-io:mainfrom
Tshepo1103-lab:tshepo/action-configurations

Conversation

@Tshepo1103-lab
Copy link
Copy Markdown
Contributor

@Tshepo1103-lab Tshepo1103-lab commented Dec 11, 2025

…logic in ActionSelect component

Summary by CodeRabbit

  • Improvements
    • Actions now display in a consistent, deterministic order across configuration UIs via a new sortOrder-based sorting and alphabetical tiebreaker.
    • Several built-in actions received explicit display order values (e.g., execute script, call API, show message, sign in, navigation actions), stabilizing menu positions.
    • Navigation option ordering adjusted for clarity.
  • Renaming
    • "API Call" renamed to "Call API".

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 11, 2025

Walkthrough

Adds optional sortOrder to configurable action descriptors and makes action selection deterministic by sorting actions by sortOrder (default 999) then by label/name. Multiple action descriptors across providers receive explicit sortOrder values and a routing migrator was added to set a default navigationType.

Changes

Cohort / File(s) Summary
Core sorting infra
shesha-reactjs/src/interfaces/configurableAction.ts, shesha-reactjs/src/designer-components/configurableActionsConfigurator/actionSelect.tsx
Add sortOrder?: number to IConfigurableActionDescriptor. actionSelect now sorts ownerActions.actions by sortOrder (missing → 999) then by label/name before building nodes.
Dynamic modal actions
shesha-reactjs/src/providers/dynamicModal/index.tsx
Assigned sortOrder to modal actions: Show Dialog (3), Close Dialog (4), Show Confirmation Dialog (7).
Routing navigation
shesha-reactjs/src/providers/shaRouting/index.tsx, shesha-reactjs/src/providers/shaRouting/actions/navigate-arguments.ts
Added sortOrder: 2 to NAVIGATE action and a migrator to set navigationType default to 'form'. Reordered navigation-type radio options (Url moved after Form).
Application configurable actions
shesha-reactjs/src/providers/sheshaApplication/configurable-actions/execute-script.ts, shesha-reactjs/src/providers/sheshaApplication/configurable-actions/api-call.ts, shesha-reactjs/src/providers/sheshaApplication/configurable-actions/show-message.ts, shesha-reactjs/src/providers/sheshaApplication/configurable-actions/execute-sign-in.ts
Assigned sortOrder values: Execute Script (1), Call API (renamed from "API Call", 5), Show Message (6), Sign In (8). No behavior changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review sorting implementation in actionSelect.tsx for stable tie-breaking and label/name resolution.
  • Verify migrator in shaRouting/index.tsx correctly upgrades existing action arguments without data loss.
  • Confirm sortOrder values are intentional and consistent across UI contexts.

Possibly related PRs

  • fix: configuration actions #3092 — Touches configurable action descriptors including dynamicModal; likely overlaps with sortOrder additions.
  • button update #3069 — Modifies sheshaApplication action definitions (e.g., show-message); may overlap with metadata changes.

Suggested reviewers

  • James-Baloyi

Poem

🐰 I hopped through lists both long and wide,
I nudged each action to its place,
With numbers first, then names to guide,
Now every button knows its space.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: adding a sortOrder property to configurable actions and implementing sorting logic in the ActionSelect component.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6947f40 and e030a0f.

📒 Files selected for processing (1)
  • shesha-reactjs/src/providers/shaRouting/index.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type instead for values with unknown types, forcing explicit type checking
Prefer type guards over type casting for type checking
Avoid monolithic types; use discriminated unions with a discriminator property instead
Leverage TypeScript to its full potential as a type system, not merely as a linter

Files:

  • shesha-reactjs/src/providers/shaRouting/index.tsx
🧠 Learnings (3)
📚 Learning: 2025-11-05T08:12:08.149Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/providers/layersProvider/index.tsx:35-39
Timestamp: 2025-11-05T08:12:08.149Z
Learning: In shesha-reactjs LayerGroupConfiguratorProvider (src/providers/layersProvider/index.tsx), the provider uses a unidirectional data flow pattern after initialization: props.items provide initial state to the reducer (line 35-38), user actions modify the reducer state internally, and changes sync back to the parent via onChange callbacks in consuming components (e.g., modal.tsx line 55-57 uses useDeepCompareEffect). Do NOT add a useEffect to sync props.items back to the reducer state, as this creates a feedback loop that breaks layer updates in the Calendar UI. The pattern is intentionally "controlled initialization, uncontrolled updates" where the provider manages state internally after the initial mount.

Applied to files:

  • shesha-reactjs/src/providers/shaRouting/index.tsx
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.

Applied to files:

  • shesha-reactjs/src/providers/shaRouting/index.tsx
📚 Learning: 2025-11-05T09:03:47.960Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/components/calendar/configurable-actions/refresh-calendar.tsx:5-18
Timestamp: 2025-11-05T09:03:47.960Z
Learning: In shesha-reactjs calendar configurable actions (src/components/calendar/configurable-actions/refresh-calendar.tsx), the useRefreshCalendarAction hook registers a configurable action with an empty dependency array []. Do NOT add setRefreshTrigger to the dependency array, as it is not stable (re-created when state.refreshTrigger changes) and would cause a registration loop. The executer uses a functional update pattern setRefreshTrigger((prev) => prev + 1) which doesn't need the current closure, so the action only needs to be registered once on mount.
<!--

Applied to files:

  • shesha-reactjs/src/providers/shaRouting/index.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-attempt
🔇 Additional comments (2)
shesha-reactjs/src/providers/shaRouting/index.tsx (2)

61-61: LGTM: sortOrder addition enables deterministic action ordering.

The addition of sortOrder: 2 to the Navigate action configuration is consistent with the PR's objective to implement deterministic action sorting.


73-73: Verify the design rationale for defaulting navigationType to 'form'.

The migrator implementation is correct: it uses nullish coalescing to preserve existing navigationType values ('form' or 'url') while providing 'form' as the default for undefined cases. However, confirm that 'form' is the intended default value for backward compatibility, as this choice should align with the expected behavior when navigationType was previously unspecified.

Migration version 0 is appropriate if this is the first migration for this action. Verify there are no existing migrations for INavigateActoinArguments that would require incrementing this version number.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Tshepo1103-lab
Copy link
Copy Markdown
Contributor Author

#4355

… update sign-in action owner"

This reverts commit 6a7a911.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a7a911 and 54a6daa.

📒 Files selected for processing (1)
  • shesha-reactjs/src/providers/sheshaApplication/configurable-actions/execute-sign-in.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type instead for values with unknown types, forcing explicit type checking
Prefer type guards over type casting for type checking
Avoid monolithic types; use discriminated unions with a discriminator property instead
Leverage TypeScript to its full potential as a type system, not merely as a linter

Files:

  • shesha-reactjs/src/providers/sheshaApplication/configurable-actions/execute-sign-in.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build-attempt

@IvanIlyichev IvanIlyichev merged commit 55ac453 into shesha-io:main Dec 18, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants