-
Notifications
You must be signed in to change notification settings - Fork 61
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
chore: remove stale flags #16039
chore: remove stale flags #16039
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. WalkthroughThis pull request involves a comprehensive cleanup of feature flags across multiple modules in the application. The changes primarily focus on removing feature flag decorators, state management related to feature flags, and simplifying route configurations. The modifications span various domains including notifications, law and order, vehicles, work machines, education, finance, health, and signature collection modules. The overall goal appears to be streamlining the application by eliminating conditional rendering and feature flag checks. Changes
Suggested labels
Suggested reviewers
Possibly related PRs
Note: No sequence diagram was generated as the changes are primarily administrative and do not introduce new complex interactions or control flows. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16039 +/- ##
==========================================
- Coverage 35.57% 35.57% -0.01%
==========================================
Files 7027 7027
Lines 150431 150423 -8
Branches 42942 42942
==========================================
- Hits 53516 53510 -6
+ Misses 96915 96913 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (23)
|
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.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
libs/portals/my-pages/information/src/components/PersonalInformation/Forms/ProfileForm/ProfileForm.tsx (1)
Line range hint
8-12
: Remove unused feature flag importsAfter removing the feature flag functionality for paper mail settings, the following imports appear to be unused and should be removed for better tree-shaking:
- import { - FeatureFlagClient, - Features, - useFeatureFlagClient, - } from '@island.is/react/feature-flags'
🧹 Nitpick comments (4)
libs/portals/my-pages/information/src/components/PersonalInformation/Forms/ProfileForm/ProfileForm.tsx (1)
Line range hint
50-291
: Consider extracting form logic into custom hooksTo improve reusability across different NextJS apps (as per coding guidelines), consider extracting the following logic into custom hooks:
- Form state management logic
- Drop modal handling logic
- IDS link generation logic
This would make the component more focused on presentation and easier to reuse across different contexts.
libs/portals/my-pages/assets/src/components/VehicleCard.tsx (1)
Line range hint
50-57
: LGTM! Consider adding type guard for requiresMileageRegistration.The simplified logic looks good. For better type safety, consider adding a type guard for the
requiresMileageRegistration
property.- vehicle.requiresMileageRegistration + 'requiresMileageRegistration' in vehicle && vehicle.requiresMileageRegistrationlibs/portals/my-pages/assets/src/screens/VehicleDetail/VehicleDetail.tsx (2)
158-159
: LGTM! Consider adding fallback for undefined mainInfo.The simplified logic looks good. For better type safety, consider adding a default value in case mainInfo is undefined.
- data?.vehiclesDetail?.mainInfo?.requiresMileageRegistration + data?.vehiclesDetail?.mainInfo?.requiresMileageRegistration ?? false
Line range hint
192-206
: Consider using a URL builder utility for type-safe route construction.The current string replacement method for constructing the URL could be made type-safer using a dedicated URL builder utility.
Consider creating a utility function:
const buildVehicleMileageUrl = (id: string): string => { return AssetsPaths.AssetsVehiclesDetailMileage.replace(':id', id) }Then use it in the component:
- id - ? AssetsPaths.AssetsVehiclesDetailMileage.replace( - ':id', - id.toString(), - ) - : '' + id ? buildVehicleMileageUrl(id) : ''
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/portals/my-pages/src/components/Header/Header.tsx
(2 hunks)libs/api/domains/law-and-order/src/lib/law-and-order.resolver.ts
(0 hunks)libs/api/domains/vehicles/src/lib/resolvers/bulkMileage.resolver.ts
(0 hunks)libs/api/domains/work-machines/src/lib/workMachines.resolver.ts
(0 hunks)libs/feature-flags/src/lib/features.ts
(0 hunks)libs/portals/my-pages/assets/src/components/VehicleCard.tsx
(2 hunks)libs/portals/my-pages/assets/src/module.tsx
(0 hunks)libs/portals/my-pages/assets/src/screens/VehicleDetail/VehicleDetail.tsx
(1 hunks)libs/portals/my-pages/education/src/module.tsx
(0 hunks)libs/portals/my-pages/finance/src/module.tsx
(0 hunks)libs/portals/my-pages/health/src/module.tsx
(0 hunks)libs/portals/my-pages/information/src/components/PersonalInformation/Forms/ProfileForm/ProfileForm.tsx
(1 hunks)libs/portals/my-pages/information/src/module.tsx
(0 hunks)libs/portals/my-pages/law-and-order/src/module.tsx
(0 hunks)libs/portals/my-pages/petitions/src/module.tsx
(0 hunks)libs/portals/my-pages/signature-collection/src/module.tsx
(0 hunks)
💤 Files with no reviewable changes (12)
- libs/api/domains/vehicles/src/lib/resolvers/bulkMileage.resolver.ts
- libs/portals/my-pages/signature-collection/src/module.tsx
- libs/portals/my-pages/information/src/module.tsx
- libs/portals/my-pages/law-and-order/src/module.tsx
- libs/portals/my-pages/education/src/module.tsx
- libs/portals/my-pages/assets/src/module.tsx
- libs/portals/my-pages/finance/src/module.tsx
- libs/portals/my-pages/petitions/src/module.tsx
- libs/feature-flags/src/lib/features.ts
- libs/api/domains/law-and-order/src/lib/law-and-order.resolver.ts
- libs/api/domains/work-machines/src/lib/workMachines.resolver.ts
- libs/portals/my-pages/health/src/module.tsx
🧰 Additional context used
📓 Path-based instructions (4)
apps/portals/my-pages/src/components/Header/Header.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/portals/my-pages/assets/src/components/VehicleCard.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/portals/my-pages/information/src/components/PersonalInformation/Forms/ProfileForm/ProfileForm.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/portals/my-pages/assets/src/screens/VehicleDetail/VehicleDetail.tsx (1)
Pattern libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (4)
apps/portals/my-pages/src/components/Header/Header.tsx (2)
23-23
: LGTM! Clean import statement.The simplified import statement correctly reflects the removal of feature flag functionality while maintaining only the necessary hooks.
106-110
: LGTM! Secure implementation of NotificationButton.The NotificationButton implementation correctly:
- Uses delegation access control for security
- Maintains proper state management
- Follows TypeScript type safety with MenuTypes
libs/portals/my-pages/information/src/components/PersonalInformation/Forms/ProfileForm/ProfileForm.tsx (2)
Line range hint
39-48
: Well-structured TypeScript props interfaceThe Props interface follows TypeScript best practices with clear type definitions and proper optional property markers.
Line range hint
281-289
: Clean implementation of paper mail sectionThe simplified implementation properly handles conditional rendering based on the
showDetails
prop while maintaining localization support and TypeScript type safety.
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.
LGTM
Feature flag cleanup
isServicePortalAirDiscountModuleEnabled
isServicePortalDisabilityLicenseEnabled
isServicePortalDocumentsV2PageEnabled
isServicePortalEducationGratuationModuleEnabled
isServicePortalFinanceHmsLoansPageEnabled
isServicePortalHealthCenterPageEnabled
isServicePortalHealthIncomePageEnabled
isServicePortalHealthMedicinePageEnabled
isServicePortalHealthOverviewPageEnabled
isServicePortalHealthPaymentPageEnabled
isServicePortalHealthRightsModuleEnabled
isServicePortalHealthTransferPageEnabled
isServicePortalHousingBenefitsPageEnabled
isServicePortalIntellectualPropertiesPageEnabled
isServicePortalLawAndOrderModuleEnabled
isServicePortalNotificationsPageEnabled
isServicePortalPaperMailSettingsEnabled
isServicePortalParliamentaryListsPageEnabled
isServicePortalPetitionsModuleEnabled
isServicePortalSecondaryEducationPageEnabled
isServicePortalSignatureCollectionEnabled
isServicePortalVehicleBulkMileagePageEnabled
isServicePortalVehicleMileagePageEnabled
isServicePortalWorkMachinesPageEnabled
isserviceportalnationalregistryv3enabled
servicePortalTransferHealthCenter
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
Release Notes
Feature Flag Removals
Component Updates
Routing Changes
These changes simplify the application's feature management and remove conditional rendering based on feature flags.