segments: add a separate validateButtonAction for next section#1385
segments: add a separate validateButtonAction for next section#1385tahini merged 1 commit intochairemobilite:mainfrom
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
WalkthroughThis PR centralizes widget factory configuration by introducing a shared Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/evolution-common/src/services/questionnaire/sections/segments/buttonSaveTripSegments.ts (1)
31-36: 🧹 Nitpick | 🔵 TrivialMinor: direct mutation of response object on line 33 is unnecessary.
segment._isNew = falsemutates the interview response in-place. Since the actual update is done viaupdateValuesbyPathon line 35, this direct mutation is a no-op side effect. Pre-existing, so no action needed now, but worth cleaning up if you're ever back in this area.Proposed cleanup
for (const segmentUuid in segments) { const segment = segments[segmentUuid]; - segment._isNew = false; const segmentPath = `${segmentsPath}.${segmentUuid}`; updateValuesbyPath[`response.${segmentPath}._isNew`] = false; }
🤖 Fix all issues with AI agents
In `@example/demo_survey/src/survey/widgets.ts`:
- Around line 137-139: Remove the stray extra semicolon after the exported
widget constants so each export ends with a single semicolon; specifically
update the lines that declare buttonSaveNextSection and buttonSaveNextSection2
(which call getButtonValidateAndGotoNextSection with widgetFactoryOptions) to
remove the trailing double semicolon.
In
`@packages/evolution-common/src/services/questionnaire/sections/common/buttonValidateAndGotoNextSection.ts`:
- Line 12: Update the JSDoc that currently mentions `validateButtonAction` to
reference the actual action used, `validateButtonActionWithCompleteSection`;
locate the JSDoc block above the `buttonValidateAndGotoNextSection` (or the
corresponding exported function) and change the description and any
`@see`/`@returns` text to mention `validateButtonActionWithCompleteSection` so
the doc matches the implementation.
...lution-common/src/services/questionnaire/sections/common/buttonValidateAndGotoNextSection.ts
Outdated
Show resolved
Hide resolved
This fixes a regression with commit f2ade4d where it was not possible to confirm a segment group because it tried to navigate to the next section, but it is not possible. It adds a separate `validateButtonActionWithCompleteSection` button action to the `buttonActions` part of the `WidgetFactoryOptions`. Thus each button can choose the appropriate action to call depending on its need.
tahini
force-pushed
the
fixSegmentsButton
branch
from
64856da to
6073ef4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/evolution-common/src/services/questionnaire/sections/segments/buttonSaveTripSegments.ts (1)
25-40: 🧹 Nitpick | 🔵 TrivialMinor:
segment._isNew = falsemutates the response object in-place.Line 33 directly mutates
segment._isNewon the interview response object, in addition to setting it viaupdateValuesbyPathon line 35. The mutation is redundant since the update path handles it. This is pre-existing, but worth noting if you revisit this area.example/demo_survey/src/survey/widgets.ts (1)
136-158: 🧹 Nitpick | 🔵 TrivialFive factory buttons now use shared options — consistent and clean.
One small inconsistency:
buttonStartNextSection(line 140-152) still referencesvalidateButtonActionWithCompleteSectiondirectly instead of going throughwidgetFactoryOptions.buttonActions.validateButtonActionWithCompleteSection. Not a big deal — especially since this demo survey is slated for replacement — but worth noting for consistency.Optional: align buttonStartNextSection with the others
export const buttonStartNextSection: any = { type: "button", path: "buttonStartNextSection", color: "green", label: { fr: "Débuter", en: "Start" }, hideWhenRefreshing: true, icon: faPlay, align: 'center', - action: validateButtonActionWithCompleteSection + action: widgetFactoryOptions.buttonActions.validateButtonActionWithCompleteSection }This would also allow removing the direct
validateButtonActionWithCompleteSectionimport on line 9.
🤖 Fix all issues with AI agents
In `@example/demo_survey/src/survey/helper.ts`:
- Around line 62-69: Use JavaScript/TypeScript object property shorthand in the
widgetFactoryOptions declaration: replace explicit longhand assignments like
getFormattedDate: getFormattedDate and validateButtonAction:
validateButtonAction with their shorthand forms, and similarly for
validateButtonActionWithCompleteSection and faCheckCircle inside iconMapper, so
the object reads more concisely while keeping the same identifiers
(widgetFactoryOptions, getFormattedDate,
validateButtonActionWithCompleteSection, validateButtonAction, iconMapper,
faCheckCircle).
| export const widgetFactoryOptions: WidgetFactoryOptions = { | ||
| getFormattedDate: getFormattedDate, | ||
| buttonActions: { | ||
| validateButtonActionWithCompleteSection: validateButtonActionWithCompleteSection, | ||
| validateButtonAction: validateButtonAction | ||
| }, | ||
| iconMapper: { 'check-circle': faCheckCircle } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Nit: consider using property shorthand.
Since the variable names match the property names, shorthand would be slightly cleaner.
✏️ Suggested shorthand
export const widgetFactoryOptions: WidgetFactoryOptions = {
- getFormattedDate: getFormattedDate,
+ getFormattedDate,
buttonActions: {
- validateButtonActionWithCompleteSection: validateButtonActionWithCompleteSection,
- validateButtonAction: validateButtonAction
+ validateButtonActionWithCompleteSection,
+ validateButtonAction
},
iconMapper: { 'check-circle': faCheckCircle }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const widgetFactoryOptions: WidgetFactoryOptions = { | |
| getFormattedDate: getFormattedDate, | |
| buttonActions: { | |
| validateButtonActionWithCompleteSection: validateButtonActionWithCompleteSection, | |
| validateButtonAction: validateButtonAction | |
| }, | |
| iconMapper: { 'check-circle': faCheckCircle } | |
| }; | |
| export const widgetFactoryOptions: WidgetFactoryOptions = { | |
| getFormattedDate, | |
| buttonActions: { | |
| validateButtonActionWithCompleteSection, | |
| validateButtonAction | |
| }, | |
| iconMapper: { 'check-circle': faCheckCircle } | |
| }; |
🤖 Prompt for AI Agents
In `@example/demo_survey/src/survey/helper.ts` around lines 62 - 69, Use
JavaScript/TypeScript object property shorthand in the widgetFactoryOptions
declaration: replace explicit longhand assignments like getFormattedDate:
getFormattedDate and validateButtonAction: validateButtonAction with their
shorthand forms, and similarly for validateButtonActionWithCompleteSection and
faCheckCircle inside iconMapper, so the object reads more concisely while
keeping the same identifiers (widgetFactoryOptions, getFormattedDate,
validateButtonActionWithCompleteSection, validateButtonAction, iconMapper,
faCheckCircle).
2 participants
This fixes a regression with commit
f2ade4d where it was not possible to confirm a segment group because it tried to navigate to the next section, but it is not possible.
It adds a separate
validateButtonActionWithCompleteSectionbutton action to thebuttonActionspart of theWidgetFactoryOptions. Thus each button can choose the appropriate action to call depending on its need.Summary by CodeRabbit
Refactor
Tests