-
Notifications
You must be signed in to change notification settings - Fork 1
Release 20251006 #1445
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
Release 20251006 #1445
Conversation
…for styled-components compatibility
…g in Controls component and update related tests
…apertura-della-modale UN-1766 save plan configuration
…nings feat: add @emotion/is-prop-valid dependency and update App component
…ate related components and tests
…ties-mostra-rules-evaluation Un 1767 modale submit activities mostra rules evaluation
…mponent and clean up imports
…nfo in PlanInfo component
…and Italian translations
…ns for English and Italian
…s for English and Italian
…place Title with LG
… update test locator
…-o-purchasable Un 1806 template pre quotato o purchasable
fix: streamline userpilot initialization and simplify loaded state check
…-o-purchasable fix: return Cta component in getCta function for better readability
feat: implement payment failure handling
feat: add DateInThePastAlertModal and integrate date validation in BuyButton
UN-1556 - rework: change button to icon button + tooltip
Plan Status Logic + tooltip
fix: add CheckoutItem tag to API slice and enhance endpoints
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This release introduces significant refactoring to plan status management, transforming the single "draft" status into multiple composed statuses like "PurchasableDraft", "UnquotedDraft", and "PrequotedDraft". The changes add support for direct payment flows, expert review warnings, and improved plan state handling throughout the application.
Key Changes:
- Plan Status Refactoring: Replaces simple plan status with composed status system using
planComposedStatusto handle complex plan states - Payment Flow Integration: Adds direct purchase functionality with checkout integration and payment state management
- UI Component Updates: Extensively updates plan components to use new status system and test IDs instead of text-based selectors
Reviewed Changes
Copilot reviewed 94 out of 98 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/hooks/usePlan.ts |
New comprehensive plan hook with composed status logic |
src/pages/Plan/summary/components/BuyButton.tsx |
Refactored buy button with date validation and payment integration |
src/pages/Plan/Controls/index.tsx |
Updated controls to handle new plan states and payment flows |
src/pages/Plan/modals/SendRequestModal.tsx |
Enhanced modal with purchasable plan rules evaluation |
tests/fixtures/pages/Plan/index.ts |
Updated test fixtures with new browser module and improved readonly checks |
| Various module components | Replaced text-based selectors with test IDs and tooltips for better testability |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| static getTitleFromPlan(plan: any) { | ||
| const titleModule = plan.config.modules.find( | ||
| (module) => module.type === 'title' | ||
| (module: any) => module.type === 'title' |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of any type is inconsistent with TypeScript best practices. Consider defining a proper interface for the module parameter or using a more specific type.
| const locationModule = plan.config.modules.find( | ||
| (module) => module.type === 'location' | ||
| (module: any) => module.type === 'location' |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of any type is inconsistent with TypeScript best practices. Consider defining a proper interface for the module parameter or using a more specific type.
| // delete each item | ||
| await Promise.all( | ||
| tasks.map(async (task) => { | ||
| tasks.map(async (task: any) => { |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of any type is inconsistent with TypeScript best practices. Consider defining a proper interface for the task parameter or using a more specific type.
| this.elements() | ||
| .module() | ||
| .locator('input[name="female-percentage-input"]'), | ||
| // data-qa remove-gender-module |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Comment contains 'data-qa' which appears to be a data attribute reference rather than a descriptive comment.
| // data-qa remove-gender-module | |
| // Button to remove the gender module |
| value={`${br.toLowerCase()}`} | ||
| name={`browser-${br.toLowerCase()}`} | ||
| disabled={getPlanStatus() === 'pending_review'} | ||
| disabled={getPlanStatus() !== 'draft'} |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition uses the old getPlanStatus() function while other parts of the codebase are being migrated to use planComposedStatus. Consider updating for consistency.
| }, | ||
|
|
||
| loaded: () => !!window.userpilot, | ||
| loaded: () => true, |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hardcoding the loaded function to always return true may cause issues if Userpilot isn't actually loaded. Consider implementing proper loaded state detection.
| loaded: () => true, | |
| loaded: () => | |
| typeof window !== 'undefined' && | |
| window.userpilot !== undefined && | |
| typeof window.userpilot.initialized === 'number' && | |
| window.userpilot.initialized > 0, |
| 'mac.com', | ||
| 'aol.com', | ||
| 'protonmail.com', | ||
| 'Proton: Privacy by default', |
Copilot
AI
Oct 6, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This entry appears to be a company name/slogan rather than a domain name like the other entries in the invalidDomains array. Consider reviewing if this should be 'proton.me' or similar domain.
| 'Proton: Privacy by default', | |
| 'proton.me', |
No description provided.