feat: WIP allow to upgrade manage forms to customized#886
Merged
Conversation
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
📝 WalkthroughWalkthroughAdds a managed sponsor-form upgrade flow: new Redux action types and thunks to fetch and upgrade managed forms, conditions the customized-form popup to use those thunks when invoked as an upgrade, and updates the forms tab to manage the upgrade state; also removes a stray debug log. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Popup as CustomizedFormPopup
participant Redux
participant API as Server
User->>Popup: open customize (upgradeManaged = true)
Popup->>Redux: dispatch getSponsorManagedForm(formId)
Redux->>API: GET /sponsors/{sponsorId}/managed-forms/{formId}?expand=...
API-->>Redux: 200 managed form payload
Redux-->>Popup: RECEIVE_SPONSOR_CUSTOMIZED_FORM (normalized)
User->>Popup: save changes
Popup->>Redux: dispatch upgradeSponsorManagedForm(entity)
Redux->>API: PUT /sponsors/{sponsorId}/managed-forms/{formId} (normalized payload)
API-->>Redux: 200 updated managed form
Redux-->>Popup: SPONSOR_MANAGED_FORMS_UPGRADED
Popup->>User: close popup (refresh lists)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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)
src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.js (1)
121-123:⚠️ Potential issue | 🟡 MinorRemove debug
console.logstatement.This appears to be a leftover debug statement that should be removed before merging.
🧹 Proposed fix
const handleManagedFormManageItems = (item) => { - console.log("Managed Form Item Edit : ", item); + // TODO: Implement managed form item navigation };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.js` around lines 121 - 123, Remove the leftover debug console.log by deleting the console.log call inside the handleManagedFormManageItems function; leave the function signature (const handleManagedFormManageItems = (item) => { ... }) and any real logic or event handling there, or replace the console.log with proper handling or a TODO comment if further work is required.
🧹 Nitpick comments (3)
src/actions/sponsor-forms-actions.js (2)
54-57: Unused action type constants.
REQUEST_SPONSOR_MANAGED_FORMandRECEIVE_SPONSOR_MANAGED_FORMare exported but never used.getSponsorManagedFormdispatchesRECEIVE_SPONSOR_CUSTOMIZED_FORMinstead ofRECEIVE_SPONSOR_MANAGED_FORM, which creates naming confusion.Either use the new action types or remove them to avoid dead code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/sponsor-forms-actions.js` around lines 54 - 57, The file defines unused constants REQUEST_SPONSOR_MANAGED_FORM and RECEIVE_SPONSOR_MANAGED_FORM while getSponsorManagedForm currently dispatches RECEIVE_SPONSOR_CUSTOMIZED_FORM, causing dead code and naming confusion; either remove REQUEST_SPONSOR_MANAGED_FORM and RECEIVE_SPONSOR_MANAGED_FORM and update any references to use the existing RECEIVE_SPONSOR_CUSTOMIZED_FORM, or (preferred) change getSponsorManagedForm to dispatch REQUEST_SPONSOR_MANAGED_FORM before the async call and RECEIVE_SPONSOR_MANAGED_FORM on success (and rename/remove RECEIVE_SPONSOR_CUSTOMIZED_FORM if it was a mistaken name) so the exported constants are used consistently (update any reducers listening for the action types accordingly).
666-689: Inconsistent naming and error handler.The function
getSponsorManagedFormfetches from/show-forms/(not/managed-forms/) and dispatchesRECEIVE_SPONSOR_CUSTOMIZED_FORM. This is confusing given the function name.Also, this uses
snackbarErrorHandlerwhile the similargetSponsorCustomizedFormusesauthErrorHandler. Verify this is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/actions/sponsor-forms-actions.js` around lines 666 - 689, The function getSponsorManagedForm is misnamed or using the wrong endpoint/action: it calls the /show-forms/ endpoint and dispatches RECEIVE_SPONSOR_CUSTOMIZED_FORM while its name implies managed-forms; change either the function name to getSponsorCustomizedForm or update the URL to /managed-forms/ and the dispatched action to the appropriate RECEIVE_SPONSOR_MANAGED_FORM to match intent. Also align the error handler with the similar function getSponsorCustomizedForm by replacing snackbarErrorHandler with authErrorHandler if authentication errors should be handled the same way, or explicitly document/keep snackbarErrorHandler if different behavior is intended; update the call sites and tests that reference getSponsorManagedForm, the endpoint, or the action constant accordingly.src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/customized-form-popup.js (1)
122-126: Incomplete PropTypes definition.Several props are used but not defined in PropTypes:
upgradeManaged,formId,entity,sponsor,summitId,summitTZ, and the action props. Consider expanding PropTypes for better documentation and runtime validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/customized-form-popup.js` around lines 122 - 126, The PropTypes for CustomizedFormPopup are incomplete: add definitions for the missing props used by the component (upgradeManaged, formId, entity, sponsor, summitId, summitTZ) and for any action props (e.g., onSave, fetchSomething, or the actual action prop names used in the component) by extending CustomizedFormPopup.propTypes to include these keys with appropriate PropTypes types (bool, string, number, object, func as appropriate) and mark required where the component assumes presence; reference the existing CustomizedFormPopup.propTypes block and the prop names used throughout the component to ensure every referenced prop is declared.
🤖 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-forms-actions.js`:
- Around line 622-651: normalizeSponsorCustomizedForm is being called without
the summitTZ argument in upgradeSponsorManagedForm; update the call to pass the
current summit timezone (e.g., use currentSummit.time_zone) so date
normalization uses the correct TZ: replace
normalizeSponsorCustomizedForm(entity) with
normalizeSponsorCustomizedForm(entity, currentSummit.time_zone) (or
currentSummit.timeZone if that’s the project convention) inside the
upgradeSponsorManagedForm function.
In
`@src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/customized-form-popup.js`:
- Around line 75-83: The useEffect in customized-form-popup.js depends on both
formId and upgradeManaged but only lists formId; update the dependency array for
the useEffect that calls getSponsorManagedForm(formId) or
getSponsorCustomizedForm(formId) to include upgradeManaged so the effect re-runs
when upgradeManaged changes (e.g., change useEffect(..., [formId]) to
useEffect(..., [formId, upgradeManaged])); also ensure
getSponsorManagedForm/getSponsorCustomizedForm are stable or added to the
dependency array if they are not memoized.
---
Outside diff comments:
In `@src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.js`:
- Around line 121-123: Remove the leftover debug console.log by deleting the
console.log call inside the handleManagedFormManageItems function; leave the
function signature (const handleManagedFormManageItems = (item) => { ... }) and
any real logic or event handling there, or replace the console.log with proper
handling or a TODO comment if further work is required.
---
Nitpick comments:
In `@src/actions/sponsor-forms-actions.js`:
- Around line 54-57: The file defines unused constants
REQUEST_SPONSOR_MANAGED_FORM and RECEIVE_SPONSOR_MANAGED_FORM while
getSponsorManagedForm currently dispatches RECEIVE_SPONSOR_CUSTOMIZED_FORM,
causing dead code and naming confusion; either remove
REQUEST_SPONSOR_MANAGED_FORM and RECEIVE_SPONSOR_MANAGED_FORM and update any
references to use the existing RECEIVE_SPONSOR_CUSTOMIZED_FORM, or (preferred)
change getSponsorManagedForm to dispatch REQUEST_SPONSOR_MANAGED_FORM before the
async call and RECEIVE_SPONSOR_MANAGED_FORM on success (and rename/remove
RECEIVE_SPONSOR_CUSTOMIZED_FORM if it was a mistaken name) so the exported
constants are used consistently (update any reducers listening for the action
types accordingly).
- Around line 666-689: The function getSponsorManagedForm is misnamed or using
the wrong endpoint/action: it calls the /show-forms/ endpoint and dispatches
RECEIVE_SPONSOR_CUSTOMIZED_FORM while its name implies managed-forms; change
either the function name to getSponsorCustomizedForm or update the URL to
/managed-forms/ and the dispatched action to the appropriate
RECEIVE_SPONSOR_MANAGED_FORM to match intent. Also align the error handler with
the similar function getSponsorCustomizedForm by replacing snackbarErrorHandler
with authErrorHandler if authentication errors should be handled the same way,
or explicitly document/keep snackbarErrorHandler if different behavior is
intended; update the call sites and tests that reference getSponsorManagedForm,
the endpoint, or the action constant accordingly.
In
`@src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/customized-form-popup.js`:
- Around line 122-126: The PropTypes for CustomizedFormPopup are incomplete: add
definitions for the missing props used by the component (upgradeManaged, formId,
entity, sponsor, summitId, summitTZ) and for any action props (e.g., onSave,
fetchSomething, or the actual action prop names used in the component) by
extending CustomizedFormPopup.propTypes to include these keys with appropriate
PropTypes types (bool, string, number, object, func as appropriate) and mark
required where the component assumes presence; reference the existing
CustomizedFormPopup.propTypes block and the prop names used throughout the
component to ensure every referenced prop is declared.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c84efb8-758a-463c-9bda-98702d39f905
📒 Files selected for processing (4)
src/actions/sponsor-forms-actions.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/customized-form-popup.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/index.jssrc/pages/sponsors/sponsor-page/tabs/sponsor-pages-tab/index.js
Comment on lines
75
to
83
| useEffect(() => { | ||
| if (formId) { | ||
| getSponsorCustomizedForm(formId); | ||
| if (upgradeManaged) { | ||
| getSponsorManagedForm(formId); | ||
| } else { | ||
| getSponsorCustomizedForm(formId); | ||
| } | ||
| } | ||
| }, [formId]); |
There was a problem hiding this comment.
Missing upgradeManaged in useEffect dependency array.
The effect uses upgradeManaged to determine which fetch function to call, but it's not included in the dependency array. If upgradeManaged changes while formId remains the same, the effect won't re-run with the correct fetch logic.
🔧 Proposed fix
useEffect(() => {
if (formId) {
if (upgradeManaged) {
getSponsorManagedForm(formId);
} else {
getSponsorCustomizedForm(formId);
}
}
- }, [formId]);
+ }, [formId, upgradeManaged]);📝 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
| useEffect(() => { | |
| if (formId) { | |
| getSponsorCustomizedForm(formId); | |
| if (upgradeManaged) { | |
| getSponsorManagedForm(formId); | |
| } else { | |
| getSponsorCustomizedForm(formId); | |
| } | |
| } | |
| }, [formId]); | |
| useEffect(() => { | |
| if (formId) { | |
| if (upgradeManaged) { | |
| getSponsorManagedForm(formId); | |
| } else { | |
| getSponsorCustomizedForm(formId); | |
| } | |
| } | |
| }, [formId, upgradeManaged]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/pages/sponsors/sponsor-page/tabs/sponsor-forms-tab/components/customized-form/customized-form-popup.js`
around lines 75 - 83, The useEffect in customized-form-popup.js depends on both
formId and upgradeManaged but only lists formId; update the dependency array for
the useEffect that calls getSponsorManagedForm(formId) or
getSponsorCustomizedForm(formId) to include upgradeManaged so the effect re-runs
when upgradeManaged changes (e.g., change useEffect(..., [formId]) to
useEffect(..., [formId, upgradeManaged])); also ensure
getSponsorManagedForm/getSponsorCustomizedForm are stable or added to the
dependency array if they are not memoized.
Signed-off-by: Tomás Castillo <tcastilloboireau@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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-forms-actions.js`:
- Around line 684-692: The GET in getSponsorManagedForm is using
snackbarErrorHandler instead of the standard authErrorHandler; update the
getRequest call that dispatches createAction(RECEIVE_SPONSOR_CUSTOMIZED_FORM)
for the URL
`${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}`
to pass authErrorHandler (matching other GETs in this file) so authentication
errors are handled consistently (e.g., 401 redirects).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7609594b-7167-4994-ae57-3a3d1b328b68
📒 Files selected for processing (1)
src/actions/sponsor-forms-actions.js
Comment on lines
+684
to
+692
| return getRequest( | ||
| null, | ||
| createAction(RECEIVE_SPONSOR_CUSTOMIZED_FORM), | ||
| `${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}`, | ||
| snackbarErrorHandler | ||
| )(params)(dispatch).then(() => { | ||
| dispatch(stopLoading()); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Inconsistent error handler for GET request.
All other GET requests in this file use authErrorHandler, but getSponsorManagedForm uses snackbarErrorHandler. This inconsistency could mask authentication errors that other GET actions would properly handle (e.g., redirect to login on 401).
🔧 Proposed fix
return getRequest(
null,
createAction(RECEIVE_SPONSOR_CUSTOMIZED_FORM),
`${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}`,
- snackbarErrorHandler
+ authErrorHandler
)(params)(dispatch).then(() => {📝 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 getRequest( | |
| null, | |
| createAction(RECEIVE_SPONSOR_CUSTOMIZED_FORM), | |
| `${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}`, | |
| snackbarErrorHandler | |
| )(params)(dispatch).then(() => { | |
| dispatch(stopLoading()); | |
| }); | |
| }; | |
| return getRequest( | |
| null, | |
| createAction(RECEIVE_SPONSOR_CUSTOMIZED_FORM), | |
| `${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}`, | |
| authErrorHandler | |
| )(params)(dispatch).then(() => { | |
| dispatch(stopLoading()); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/actions/sponsor-forms-actions.js` around lines 684 - 692, The GET in
getSponsorManagedForm is using snackbarErrorHandler instead of the standard
authErrorHandler; update the getRequest call that dispatches
createAction(RECEIVE_SPONSOR_CUSTOMIZED_FORM) for the URL
`${window.PURCHASES_API_URL}/api/v1/summits/${currentSummit.id}/show-forms/${formId}`
to pass authErrorHandler (matching other GETs in this file) so authentication
errors are handled consistently (e.g., 401 redirects).
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
2 participants
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/86b66n5jn
Signed-off-by: Tomás Castillo tcastilloboireau@gmail.com
Summary by CodeRabbit
New Features
Bug Fixes