Open
Conversation
📝 WalkthroughWalkthroughChanges extend sponsor page management by introducing customized page create/edit workflows. New action creators handle fetching and saving single customized pages with timezone-aware normalization. The PageTemplatePopup component gains allowed addons selection UI. SponsorPagesTab integrates these actions with corresponding state management. Reducer tracks current edit page and global sponsorships. Translations and tests support the new functionality. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as SponsorPagesTab UI
participant Action as Actions
participant API as API/Server
participant Store as Redux Store
participant Component as PageTemplatePopup
User->>UI: Click "New Page" or Edit icon
UI->>Action: Dispatch getSponsorCustomizedPage (on edit)<br/>or skip (on create)
alt Edit Flow
Action->>API: Fetch single customized page<br/>with allowed_add_ons
API-->>Action: Return page data
Action->>Store: Dispatch RECEIVE_SPONSOR_CUSTOMIZED_PAGE
Store->>UI: Update currentEditPage state
end
UI->>Component: Render PageTemplatePopup<br/>with summitId, sponsorId, sponsorshipIds
Component->>Action: Query sponsor addons
Action->>API: Fetch allowed addons list
API-->>Component: Return addons data
Component->>Component: Render addons selection
User->>Component: Fill form & save
Component->>UI: onSave(pageData)
UI->>Action: Dispatch saveSponsorCustomizedPage
Action->>API: POST/PUT customized page
API-->>Action: Return saved page
Action->>Store: Dispatch SPONSOR_CUSTOMIZED_PAGE_ADDED/UPDATED
Action->>UI: Show snackbar success
UI->>Action: Dispatch getSponsorCustomizedPages<br/>(refresh list)
Action->>API: Fetch customized pages list
API-->>Store: Update pages list
UI->>UI: Close popup, refresh view
UI->>Action: Dispatch resetSponsorPage
Store->>Store: Reset currentEditPage
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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)
📝 Coding Plan
Comment Tip CodeRabbit can suggest fixes for GitHub Check annotations.Configure the |
tomrndom
force-pushed
the
feature/sponsor-tab-custom-add
branch
from
734f0dc to
17f6068
Compare
tomrndom
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/pages/sponsors-global/page-templates/page-template-popup/index.js (1)
236-251: Deeply nested property access ingetGroupLabelmay be fragile.Line 245 accesses
addon.sponsorship.type.type.namewhich is four levels deep. If any intermediate property is missing, this will throw a runtime error.Consider adding optional chaining for defensive coding:
Proposed defensive fix
- getGroupLabel={(addon) => addon.sponsorship.type.type.name} + getGroupLabel={(addon) => addon.sponsorship?.type?.type?.name ?? ""}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js` around lines 236 - 251, Replace the fragile deep property access used in the MuiFormikSelectGroup prop getGroupLabel (currently referencing addon.sponsorship.type.type.name) with a defensive access pattern using optional chaining and a safe fallback (e.g., empty string or a sensible label) so missing intermediate properties won't throw; update the getGroupLabel handler in the component where MuiFormikSelectGroup is declared to return addon.sponsorship?.type?.type?.name ?? '' (or another default) instead of directly accessing addon.sponsorship.type.type.name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/actions/sponsor-pages-actions.js`:
- Around line 325-339: The POST branch calling postRequest with
createAction(SPONSOR_CUSTOMIZED_PAGE_ADDED) should dispatch stopLoading() in a
.finally() so the loading spinner is cleared on errors; update the promise chain
that currently calls dispatch(stopLoading()) inside .then() to instead attach a
.finally(() => dispatch(stopLoading())) while keeping the snackbarSuccessHandler
in .then() and the snackbarErrorHandler passed to postRequest unchanged
(symbols: postRequest, createAction(SPONSOR_CUSTOMIZED_PAGE_ADDED),
snackbarSuccessHandler, snackbarErrorHandler, stopLoading).
In `@src/components/inputs/formik-text-editor.js`:
- Around line 16-18: helpers.setValue and helpers.setTouched are being called
with the field name as the first argument which is incorrect because useField's
helpers are already scoped to the field; change the calls to pass only the value
and touched boolean respectively (replace helpers.setValue(name, stringValue)
with helpers.setValue(stringValue) and replace helpers.setTouched(name, true)
with helpers.setTouched(true)), ensuring you still reference the existing
variables stringValue and name where needed elsewhere.
In `@src/pages/sponsors/sponsor-pages-tab/index.js`:
- Around line 286-288: The code assumes
sponsor.sponsorships_collection.sponsorships exists before mapping and will
throw if either is undefined; update the computation of sponsorshipIds to
defensively handle missing properties by checking
sponsor.sponsorships_collection and sponsor.sponsorships_collection.sponsorships
(or using optional chaining and a default empty array) before calling .map, e.g.
compute sponsorshipIds from (sponsor.sponsorships_collection?.sponsorships ||
[]) to ensure sponsorshipIds is always an array; apply this change where
sponsorshipIds is declared so downstream uses of sponsorshipIds are safe.
- Line 234: Remove the stray extra semicolon at the end of the promise chain:
change the trailing "});; " to a single "});" in the block that calls
setOpenPopup(null) in the promise.finally callback (look for the finally(() =>
setOpenPopup(null)) usage), so the statement ends with a single semicolon.
---
Nitpick comments:
In `@src/pages/sponsors-global/page-templates/page-template-popup/index.js`:
- Around line 236-251: Replace the fragile deep property access used in the
MuiFormikSelectGroup prop getGroupLabel (currently referencing
addon.sponsorship.type.type.name) with a defensive access pattern using optional
chaining and a safe fallback (e.g., empty string or a sensible label) so missing
intermediate properties won't throw; update the getGroupLabel handler in the
component where MuiFormikSelectGroup is declared to return
addon.sponsorship?.type?.type?.name ?? '' (or another default) instead of
directly accessing addon.sponsorship.type.type.name.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/actions/sponsor-pages-actions.jssrc/components/inputs/formik-text-editor.jssrc/components/mui/dropdown-checkbox.jssrc/i18n/en.jsonsrc/pages/sponsors-global/page-templates/page-template-popup/index.jssrc/pages/sponsors/sponsor-forms-tab/components/add-sponsor-form-template-popup/index.jssrc/pages/sponsors/sponsor-pages-tab/index.jssrc/reducers/sponsors/sponsor-page-pages-list-reducer.jssrc/utils/constants.js
Comment on lines
+325
to
+339
| return postRequest( | ||
| null, | ||
| createAction(SPONSOR_CUSTOMIZED_PAGE_ADDED), | ||
| `${window.SPONSOR_PAGES_API_URL}/api/v1/summits/${currentSummit.id}/sponsors/${sponsorId}/sponsor-pages`, | ||
| normalizedEntity, | ||
| snackbarErrorHandler | ||
| )(params)(dispatch).then(() => { | ||
| dispatch(stopLoading()); | ||
| dispatch( | ||
| snackbarSuccessHandler({ | ||
| title: T.translate("general.success"), | ||
| html: T.translate("edit_sponsor.pages_tab.custom_page_created") | ||
| }) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Missing .finally() on POST request will leave loading spinner on error.
The PUT request (lines 304-322) correctly uses .finally() to call stopLoading(), but the POST request dispatches stopLoading() inside .then(). If the POST request fails, the loading spinner will remain indefinitely.
Proposed fix: Move stopLoading to a .finally() block
return postRequest(
null,
createAction(SPONSOR_CUSTOMIZED_PAGE_ADDED),
`${window.SPONSOR_PAGES_API_URL}/api/v1/summits/${currentSummit.id}/sponsors/${sponsorId}/sponsor-pages`,
normalizedEntity,
snackbarErrorHandler
- )(params)(dispatch).then(() => {
- dispatch(stopLoading());
+ )(params)(dispatch)
+ .then(() => {
dispatch(
snackbarSuccessHandler({
title: T.translate("general.success"),
html: T.translate("edit_sponsor.pages_tab.custom_page_created")
})
);
- });
+ })
+ .finally(() => {
+ dispatch(stopLoading());
+ });
};📝 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.
Suggested change
| return postRequest( | |
| null, | |
| createAction(SPONSOR_CUSTOMIZED_PAGE_ADDED), | |
| `${window.SPONSOR_PAGES_API_URL}/api/v1/summits/${currentSummit.id}/sponsors/${sponsorId}/sponsor-pages`, | |
| normalizedEntity, | |
| snackbarErrorHandler | |
| )(params)(dispatch).then(() => { | |
| dispatch(stopLoading()); | |
| dispatch( | |
| snackbarSuccessHandler({ | |
| title: T.translate("general.success"), | |
| html: T.translate("edit_sponsor.pages_tab.custom_page_created") | |
| }) | |
| ); | |
| }); | |
| return postRequest( | |
| null, | |
| createAction(SPONSOR_CUSTOMIZED_PAGE_ADDED), | |
| `${window.SPONSOR_PAGES_API_URL}/api/v1/summits/${currentSummit.id}/sponsors/${sponsorId}/sponsor-pages`, | |
| normalizedEntity, | |
| snackbarErrorHandler | |
| )(params)(dispatch) | |
| .then(() => { | |
| dispatch( | |
| snackbarSuccessHandler({ | |
| title: T.translate("general.success"), | |
| html: T.translate("edit_sponsor.pages_tab.custom_page_created") | |
| }) | |
| ); | |
| }) | |
| .finally(() => { | |
| dispatch(stopLoading()); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/sponsor-pages-actions.js` around lines 325 - 339, The POST branch
calling postRequest with createAction(SPONSOR_CUSTOMIZED_PAGE_ADDED) should
dispatch stopLoading() in a .finally() so the loading spinner is cleared on
errors; update the promise chain that currently calls dispatch(stopLoading())
inside .then() to instead attach a .finally(() => dispatch(stopLoading())) while
keeping the snackbarSuccessHandler in .then() and the snackbarErrorHandler
passed to postRequest unchanged (symbols: postRequest,
createAction(SPONSOR_CUSTOMIZED_PAGE_ADDED), snackbarSuccessHandler,
snackbarErrorHandler, stopLoading).
Comment on lines
+16
to
+18
| helpers.setValue(name, stringValue); | ||
| }} | ||
| onBlur={() => setFieldTouched(name, true)} | ||
| error={touched?.[name] && errors?.[name] ? errors?.[name] : ""} | ||
| onBlur={() => helpers.setTouched(name, true)} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Formik useField API helpers.setValue helpers.setTouched signature documentation
💡 Result:
In Formik, useField returns a 3-tuple: [field, meta, helpers], where helpers is FieldHelperProps (aka “FieldHelpers”).[1][2]
helpers method signatures (as documented):
helpers.setValue(value: any, shouldValidate?: boolean): void[1]helpers.setTouched(value: boolean, shouldValidate?: boolean): void[1]helpers.setError(value: any): void[1]
Notes:
shouldValidatelets you explicitly skip validation by passingfalse(otherwise it followsvalidateOnChange/validateOnBlur).[1]- The helpers object was added to
useFieldso you can callhelpers.setValue(...)/helpers.setTouched(...)instead offormik.setFieldValue(name, ...)/formik.setFieldTouched(name, ...).[2]
Sources: [1] https://formik.bootcss.com/api/useField | [2] https://newreleases.io/project/yarn/formik/release/2.1.0
Fix useField helper calls — current arguments are incorrect.
On Lines 16 and 18, name is passed into helpers.setValue and helpers.setTouched. The useField helpers are field-scoped, so they take the value and touched flag directly without a field name. As written, helpers.setValue(name, stringValue) sets the field to the name string instead of the value, and helpers.setTouched(name, true) passes a string where a boolean is expected, corrupting form state.
Proposed fix
- helpers.setValue(name, stringValue);
+ helpers.setValue(stringValue);
...
- onBlur={() => helpers.setTouched(name, true)}
+ onBlur={() => helpers.setTouched(true)}📝 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.
Suggested change
| helpers.setValue(name, stringValue); | |
| }} | |
| onBlur={() => setFieldTouched(name, true)} | |
| error={touched?.[name] && errors?.[name] ? errors?.[name] : ""} | |
| onBlur={() => helpers.setTouched(name, true)} | |
| helpers.setValue(stringValue); | |
| }} | |
| onBlur={() => helpers.setTouched(true)} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/inputs/formik-text-editor.js` around lines 16 - 18,
helpers.setValue and helpers.setTouched are being called with the field name as
the first argument which is incorrect because useField's helpers are already
scoped to the field; change the calls to pass only the value and touched boolean
respectively (replace helpers.setValue(name, stringValue) with
helpers.setValue(stringValue) and replace helpers.setTouched(name, true) with
helpers.setTouched(true)), ensuring you still reference the existing variables
stringValue and name where needed elsewhere.
| order, | ||
| orderDir | ||
| ); | ||
| }).finally(() => setOpenPopup(null));; |
There was a problem hiding this comment.
Minor: Double semicolon.
Line 234 has }).finally(() => setOpenPopup(null));; with an extra semicolon. While JavaScript handles this gracefully, it should be cleaned up.
Fix
- }).finally(() => setOpenPopup(null));;
+ }).finally(() => setOpenPopup(null));📝 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.
Suggested change
| }).finally(() => setOpenPopup(null));; | |
| }).finally(() => setOpenPopup(null)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors/sponsor-pages-tab/index.js` at line 234, Remove the stray
extra semicolon at the end of the promise chain: change the trailing "});; " to
a single "});" in the block that calls setOpenPopup(null) in the promise.finally
callback (look for the finally(() => setOpenPopup(null)) usage), so the
statement ends with a single semicolon.
Comment on lines
+286
to
+288
| const sponsorshipIds = sponsor.sponsorships_collection.sponsorships.map( | ||
| (e) => e.id | ||
| ); |
There was a problem hiding this comment.
Add defensive checks for nested property access.
Accessing sponsor.sponsorships_collection.sponsorships assumes all properties exist. If sponsorships_collection or its sponsorships array is missing, this will throw a runtime error.
Proposed defensive fix
- const sponsorshipIds = sponsor.sponsorships_collection.sponsorships.map(
- (e) => e.id
- );
+ const sponsorshipIds =
+ sponsor.sponsorships_collection?.sponsorships?.map((e) => e.id) ?? [];📝 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.
Suggested change
| const sponsorshipIds = sponsor.sponsorships_collection.sponsorships.map( | |
| (e) => e.id | |
| ); | |
| const sponsorshipIds = | |
| sponsor.sponsorships_collection?.sponsorships?.map((e) => e.id) ?? []; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors/sponsor-pages-tab/index.js` around lines 286 - 288, The
code assumes sponsor.sponsorships_collection.sponsorships exists before mapping
and will throw if either is undefined; update the computation of sponsorshipIds
to defensively handle missing properties by checking
sponsor.sponsorships_collection and sponsor.sponsorships_collection.sponsorships
(or using optional chaining and a default empty array) before calling .map, e.g.
compute sponsorshipIds from (sponsor.sponsorships_collection?.sponsorships ||
[]) to ensure sponsorshipIds is always an array; apply this change where
sponsorshipIds is declared so downstream uses of sponsorshipIds are safe.
8083abd to
cfe03af
Compare
tomrndom
force-pushed
the
feature/sponsor-tab-custom-add
branch
from
bda0423 to
9e4d822
Compare
tomrndom
changed the base branch from
feature/sponsor-page-tab-managed-popup
to
master
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
tomrndom
force-pushed
the
feature/sponsor-tab-custom-add
branch
from
9e4d822 to
5c1373e
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/actions/sponsor-pages-actions.js (1)
144-152:⚠️ Potential issue | 🟠 MajorUse
.finally()for these fetch lifecycles.
stopLoading()only runs on success here. If any of these GETs reject, the sponsor pages tab stays stuck in loading state. This needs the same cleanup pattern at Line 150, Line 266, and Line 294.🔧 Suggested fix
- )(params)(dispatch).then(() => { - dispatch(stopLoading()); - }); + )(params)(dispatch).finally(() => { + dispatch(stopLoading()); + });Also applies to: 259-267, 288-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/sponsor-pages-actions.js` around lines 144 - 152, The stopLoading() dispatch is only called inside the .then() success path for the getRequest call (the getRequest(createAction(REQUEST_SPONSOR_MANAGED_PAGES), createAction(RECEIVE_SPONSOR_MANAGED_PAGES), ..., snackbarErrorHandler, ...)(params)(dispatch)), so failures leave the UI stuck loading; change the promise handling to call dispatch(stopLoading()) in a .finally() block instead of only in .then() and apply the same .finally() pattern to the other two similar calls around Line 266 and Line 294 to ensure cleanup runs on both success and error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/actions/sponsor-pages-actions.js`:
- Around line 281-286: The edit flow is dropping and then overwriting the page's
apply_to_all_add_ons flag; include apply_to_all_add_ons in the params object
used to fetch page details (add "apply_to_all_add_ons" to the fields string
alongside id,code,name,allowed_add_ons,…) and when populating the edit
form/state do not unconditionally set apply_to_all_add_ons to false—read and
preserve the value from the fetched page object (the response used by the same
code that builds params and the state used later when saving), and ensure the
save path uses that preserved value unless the user explicitly changes it.
In `@src/i18n/en.json`:
- Around line 2491-2492: Update the two i18n keys custom_page_saved and
custom_page_created in en.json to match neighboring success message style by
appending "successfully." so they read "Custom Sponsor Page saved successfully."
and "Custom Sponsor Page created successfully." to ensure consistent UI copy.
- Line 6: The "empty_list" i18n string is grammatically incorrect; update the
value for the "empty_list" key to a correct user-facing sentence (e.g., "There
are no {item} that match your criteria." or "There are no {item} matching your
criteria.") in src/i18n/en.json by replacing the current malformed text
associated with "empty_list".
In `@src/pages/sponsors/sponsor-pages-tab/index.js`:
- Around line 209-219: The refresh after saving in
handleSaveManagedPageFromTemplate (and the similar save handler) drops the
managedPages.hideArchived flag, causing archived filters to be lost; update the
calls to getSponsorManagedPages to pass managedPages.hideArchived as the final
argument (i.e., include the hideArchived value from the managedPages object when
invoking getSponsorManagedPages after saveSponsorManagedPage resolves) so the
table refresh preserves the user's archived filter.
- Around line 209-220: The save flow closes the popup in .finally() causing the
dialog to close on failure and leaves edit state populated; change
handleSaveManagedPageFromTemplate so saveSponsorManagedPage(entity).then(...)
handles success by invoking getSponsorManagedPages(...), then calls
setOpenPopup(null) and clears the edit state (e.g., setCurrentEditPage(null) or
equivalent) inside that .then; remove the .finally() teardown so failures do not
close the popup (optionally add a .catch() to surface errors but do not clear
popup or currentEditPage on error). Apply the same change to the other
customized save handler that currently uses .finally().
---
Outside diff comments:
In `@src/actions/sponsor-pages-actions.js`:
- Around line 144-152: The stopLoading() dispatch is only called inside the
.then() success path for the getRequest call (the
getRequest(createAction(REQUEST_SPONSOR_MANAGED_PAGES),
createAction(RECEIVE_SPONSOR_MANAGED_PAGES), ..., snackbarErrorHandler,
...)(params)(dispatch)), so failures leave the UI stuck loading; change the
promise handling to call dispatch(stopLoading()) in a .finally() block instead
of only in .then() and apply the same .finally() pattern to the other two
similar calls around Line 266 and Line 294 to ensure cleanup runs on both
success and error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c942abc2-4efa-4ce4-a7a9-32dccd464d96
📒 Files selected for processing (7)
src/actions/sponsor-pages-actions.jssrc/i18n/en.jsonsrc/pages/sponsors-global/page-templates/page-template-popup/index.jssrc/pages/sponsors/sponsor-forms-tab/components/add-sponsor-form-template-popup/index.jssrc/pages/sponsors/sponsor-pages-tab/__tests__/sponsor-pages-tab.test.jssrc/pages/sponsors/sponsor-pages-tab/index.jssrc/reducers/sponsors/sponsor-page-pages-list-reducer.js
Comment on lines
+281
to
+286
| const params = { | ||
| fields: | ||
| "id,code,name,allowed_add_ons,is_archived,modules,allowed_add_ons.name,allowed_add_ons.id", | ||
| expand: "allowed_add_ons", | ||
| access_token: accessToken | ||
| }; |
There was a problem hiding this comment.
Preserve apply_to_all_add_ons when editing a customized page.
Line 359 resets the flag to false, and the detail fetch at Line 282 never asks for its current value. An untouched edit of a page that currently applies to all add-ons will therefore be saved back as “not all add-ons,” and the popup cannot reflect the existing “All” state either.
🔧 Suggested fix
const params = {
fields:
- "id,code,name,allowed_add_ons,is_archived,modules,allowed_add_ons.name,allowed_add_ons.id",
+ "id,code,name,apply_to_all_add_ons,allowed_add_ons,is_archived,modules,allowed_add_ons.name,allowed_add_ons.id",
expand: "allowed_add_ons",
access_token: accessToken
};
...
const normalizedEntity = {
...entity,
- apply_to_all_add_ons: false
+ apply_to_all_add_ons: entity.apply_to_all_add_ons ?? false
};
- if (entity.allowed_add_ons.includes("all")) {
+ if (entity.allowed_add_ons?.includes("all") || entity.apply_to_all_add_ons) {
normalizedEntity.apply_to_all_add_ons = true;
normalizedEntity.allowed_add_ons = [];
} else {
- normalizedEntity.allowed_add_ons = entity.allowed_add_ons.map((e) => e.id);
+ normalizedEntity.allowed_add_ons = (entity.allowed_add_ons ?? []).map(
+ (e) => e.id
+ );
}Also applies to: 356-367
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/sponsor-pages-actions.js` around lines 281 - 286, The edit flow
is dropping and then overwriting the page's apply_to_all_add_ons flag; include
apply_to_all_add_ons in the params object used to fetch page details (add
"apply_to_all_add_ons" to the fields string alongside
id,code,name,allowed_add_ons,…) and when populating the edit form/state do not
unconditionally set apply_to_all_add_ons to false—read and preserve the value
from the fetched page object (the response used by the same code that builds
params and the state used later when saving), and ensure the save path uses that
preserved value unless the user explicitly changes it.
| "user_not_set": "Hold on. Can not get any valid user.", | ||
| "session_expired": "Hold on. Your session expired!.", | ||
| "empty_list": "there not are any item that match your criteria.", | ||
| "empty_list": "there not are any {item} that match your criteria.", |
There was a problem hiding this comment.
Fix the empty-state copy grammar.
Line 6 is user-facing and currently ungrammatical ("there not are any {item}...").
Proposed text fix
- "empty_list": "there not are any {item} that match your criteria.",
+ "empty_list": "There are no {item} that match your criteria.",📝 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.
Suggested change
| "empty_list": "there not are any {item} that match your criteria.", | |
| "empty_list": "There are no {item} that match your criteria.", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/i18n/en.json` at line 6, The "empty_list" i18n string is grammatically
incorrect; update the value for the "empty_list" key to a correct user-facing
sentence (e.g., "There are no {item} that match your criteria." or "There are no
{item} matching your criteria.") in src/i18n/en.json by replacing the current
malformed text associated with "empty_list".
Comment on lines
+2491
to
+2492
| "custom_page_saved": "Custom Sponsor Page saved", | ||
| "custom_page_created": "Custom Sponsor Page created" |
There was a problem hiding this comment.
Keep success message style consistent with neighboring strings.
Lines 2491-2492 use a different style than nearby success messages (missing period and “successfully” phrasing), which creates inconsistent UI copy.
Proposed consistency fix
- "custom_page_saved": "Custom Sponsor Page saved",
- "custom_page_created": "Custom Sponsor Page created"
+ "custom_page_saved": "Custom Sponsor Page saved successfully.",
+ "custom_page_created": "Custom Sponsor Page created successfully."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/i18n/en.json` around lines 2491 - 2492, Update the two i18n keys
custom_page_saved and custom_page_created in en.json to match neighboring
success message style by appending "successfully." so they read "Custom Sponsor
Page saved successfully." and "Custom Sponsor Page created successfully." to
ensure consistent UI copy.
Comment on lines
209
to
+219
| const handleSaveManagedPageFromTemplate = (entity) => { | ||
| saveSponsorManagedPage(entity) | ||
| .then(() => getSponsorManagedPages()) | ||
| .finally(() => setOpenPopup(null)); | ||
| .then(() => { | ||
| const { perPage, order, orderDir } = managedPages; | ||
| getSponsorManagedPages( | ||
| term, | ||
| DEFAULT_CURRENT_PAGE, | ||
| perPage, | ||
| order, | ||
| orderDir | ||
| ); |
There was a problem hiding this comment.
Preserve hideArchived when refreshing after save.
Both refresh calls drop the last hideArchived argument. If the user is filtering archived pages out, a successful save immediately reloads the tables with archived rows visible again.
🔧 Suggested fix
getSponsorManagedPages(
term,
DEFAULT_CURRENT_PAGE,
perPage,
order,
- orderDir
+ orderDir,
+ hideArchived
);
...
getSponsorCustomizedPages(
term,
DEFAULT_CURRENT_PAGE,
perPage,
order,
- orderDir
+ orderDir,
+ hideArchived
);Also applies to: 223-233
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors/sponsor-pages-tab/index.js` around lines 209 - 219, The
refresh after saving in handleSaveManagedPageFromTemplate (and the similar save
handler) drops the managedPages.hideArchived flag, causing archived filters to
be lost; update the calls to getSponsorManagedPages to pass
managedPages.hideArchived as the final argument (i.e., include the hideArchived
value from the managedPages object when invoking getSponsorManagedPages after
saveSponsorManagedPage resolves) so the table refresh preserves the user's
archived filter.
Comment on lines
209
to
+220
| const handleSaveManagedPageFromTemplate = (entity) => { | ||
| saveSponsorManagedPage(entity) | ||
| .then(() => getSponsorManagedPages()) | ||
| .finally(() => setOpenPopup(null)); | ||
| .then(() => { | ||
| const { perPage, order, orderDir } = managedPages; | ||
| getSponsorManagedPages( | ||
| term, | ||
| DEFAULT_CURRENT_PAGE, | ||
| perPage, | ||
| order, | ||
| orderDir | ||
| ); | ||
| }).finally(() => setOpenPopup(null)); |
There was a problem hiding this comment.
Move popup teardown out of .finally(), and clear edit state after a successful customized save.
Line 220 and Line 234 run on both success and failure, so a rejected save closes the dialog and drops the user’s work. In the customized flow, currentEditPage also stays populated after success, so the next “New page” can reopen with the previous id and go down the update path again.
🔧 Suggested fix
const handleSaveManagedPageFromTemplate = (entity) => {
saveSponsorManagedPage(entity)
.then(() => {
const { perPage, order, orderDir } = managedPages;
getSponsorManagedPages(
term,
DEFAULT_CURRENT_PAGE,
perPage,
order,
orderDir
);
- }).finally(() => setOpenPopup(null));
+ setOpenPopup(null);
+ });
};
const handleSaveCustomizedPage = (entity) => {
saveSponsorCustomizedPage(entity)
.then(() => {
const { perPage, order, orderDir } = customizedPages;
getSponsorCustomizedPages(
term,
DEFAULT_CURRENT_PAGE,
perPage,
order,
orderDir
);
- }).finally(() => setOpenPopup(null));;
+ resetSponsorPage();
+ setOpenPopup(null);
+ });
};Also applies to: 223-239
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/sponsors/sponsor-pages-tab/index.js` around lines 209 - 220, The
save flow closes the popup in .finally() causing the dialog to close on failure
and leaves edit state populated; change handleSaveManagedPageFromTemplate so
saveSponsorManagedPage(entity).then(...) handles success by invoking
getSponsorManagedPages(...), then calls setOpenPopup(null) and clears the edit
state (e.g., setCurrentEditPage(null) or equivalent) inside that .then; remove
the .finally() teardown so failures do not close the popup (optionally add a
.catch() to surface errors but do not clear popup or currentEditPage on error).
Apply the same change to the other customized save handler that currently uses
.finally().
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
ref:
https://app.clickup.com/t/86b7991b4
https://app.clickup.com/t/86b7991b8
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes