fix(service-portal): company route signature collections #16382
Conversation
albinagu
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16382 +/- ##
==========================================
- Coverage 36.75% 36.75% -0.01%
==========================================
Files 6804 6804
Lines 140909 140906 -3
Branches 40137 40137
==========================================
- Hits 51785 51783 -2
+ Misses 89124 89123 -1 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 21 Total Test Services: 0 Failed, 21 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
WalkthroughThe changes in this pull request introduce enhancements to the navigation structure within the service portal. A new navigation item, Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
|
![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
🧹 Outside diff range and nitpick comments (2)
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (1)
114-118: LGTM: Navigation logic improvement enhances flexibility.The conditional logic to determine the navigation path based on the current location improves the component's flexibility and reusability. This change allows the component to adapt to different contexts (company vs. individual view), adhering to the guideline of reusability across different NextJS apps.
Consider extracting the path determination logic into a separate constant for improved readability:
const navigationPath = location.pathname.includes('fyrirtaeki') ? SignatureCollectionPaths.CompanyViewParliamentaryList : SignatureCollectionPaths.ViewParliamentaryList; navigate( navigationPath.replace(':id', list.id), { state: { collectionId: currentCollection?.id || '', }, }, );This minor refactoring would make the code slightly more readable and easier to maintain.
libs/service-portal/signature-collection/src/module.tsx (1)
Line range hint
23-32: Fix Inconsistent Route Name for General PetitionsThe route for General Petitions is using the
namepropertym.signatureCollectionParliamentaryLists, which is intended for Parliamentary Lists. This could cause confusion in the navigation labels and affect user experience.Please update the
nameproperty to use the appropriate message key for General Petitions, such asm.signatureCollectionGeneralPetitions.Apply this diff to correct the
nameproperty:{ // General Petitions - name: m.signatureCollectionParliamentaryLists, + name: m.signatureCollectionGeneralPetitions, path: SignatureCollectionPaths.RootPath, enabled: userInfo.scopes.includes(ApiScope.signatureCollection), element: ( /* Default path to general petitions since these are always ongoing */ <Navigate to={SignatureCollectionPaths.GeneralPetitions} replace /> ), },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
- apps/portals/my-pages/src/lib/masterNavigation.ts (2 hunks)
- libs/service-portal/information/src/lib/navigation.ts (3 hunks)
- libs/service-portal/signature-collection/src/index.ts (1 hunks)
- libs/service-portal/signature-collection/src/lib/navigation.ts (1 hunks)
- libs/service-portal/signature-collection/src/lib/paths.ts (1 hunks)
- libs/service-portal/signature-collection/src/module.tsx (4 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (3 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
apps/portals/my-pages/src/lib/masterNavigation.ts (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/service-portal/information/src/lib/navigation.ts (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/service-portal/signature-collection/src/index.ts (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/service-portal/signature-collection/src/lib/navigation.ts (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/service-portal/signature-collection/src/lib/paths.ts (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/service-portal/signature-collection/src/module.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/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.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 (15)
libs/service-portal/signature-collection/src/index.ts (1)
3-3: 🛠️ Refactor suggestionConsider using named exports for better tree-shaking.
The addition of
export * from './lib/navigation'enhances the module's interface by including all exports from the navigation module. This practice aligns with the coding guidelines for reusability across NextJS apps and maintains TypeScript type safety.However, for optimal tree-shaking and bundling, consider using named exports instead of the wildcard export. This approach allows for more precise control over the public API and can lead to better dead code elimination during bundling.
Consider refactoring to use named exports:
export { NavigationItem, generateNavigationItems, // ... other specific exports } from './lib/navigation';This change would require knowledge of the specific exports from the navigation module. Could you provide the contents of the
./lib/navigationfile so we can suggest a more precise export statement?To ensure this change doesn't introduce any breaking changes, let's verify the usage of the navigation exports:
This will help us understand how the navigation exports are being used across the project.
✅ Verification successful
Wildcard export is appropriate given the single export in
navigation.ts.The current usage of
export * from './lib/navigation'inindex.tseffectively re-exports the sole named exportcompanySignatureCollectionNavigation. This approach aligns with the module's structure and does not negatively impact tree-shaking or bundling.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for imports from the signature-collection module rg --type typescript "from ['\"]@island\.is/service-portal/signature-collection['\"]" -A 5Length of output: 131
Script:
#!/bin/bash # Search for imports from the signature-collection module using the correct TypeScript file type rg --type ts "from ['\"]@island\.is/service-portal/signature-collection['\"]" -A 5Length of output: 1738
Script:
#!/bin/bash # List all named exports from ./lib/navigation.ts rg 'export\s+(const|function|class|interface|type)\s+' libs/service-portal/signature-collection/src/lib/navigation.tsLength of output: 195
libs/service-portal/signature-collection/src/lib/navigation.ts (1)
1-12: LGTM! The code adheres to the coding guidelines.The implementation of
companySignatureCollectionNavigationlooks good and follows the required guidelines:
- Reusability: This constant can be easily imported and used across different NextJS apps within the project.
- TypeScript usage: The constant is correctly typed as
PortalNavigationItem, which enhances type safety.- Tree-shaking: The use of named imports and exports facilitates effective tree-shaking.
Additional positive observations:
- The code structure is clean and well-organized.
- Localization is properly implemented using the
mobject for internationalized strings.- Naming conventions are clear and descriptive.
libs/service-portal/signature-collection/src/lib/paths.ts (1)
9-12: LGTM! New company-specific paths added consistently.The new enum entries for company-specific parliamentary signature collections are well-structured and consistent with the existing patterns. They maintain the reusability of the
SignatureCollectionPathsenum across different NextJS apps while leveraging TypeScript for type safety.A few observations:
- The new paths use '/fyrirtaeki/listar' as the base, clearly differentiating company-specific routes from individual user routes.
- The naming convention follows the established pattern, enhancing code readability and maintainability.
- The TypeScript enum ensures type safety when using these paths throughout the application.
apps/portals/my-pages/src/lib/masterNavigation.ts (3)
25-25: LGTM: New import statement is consistent with existing patterns.The import of
companySignatureCollectionNavigationfollows the established conventions for importing navigation items in this file. It maintains consistency in naming and module path structure.
25-25: Summary: Navigation structure enhanced with company signature collection.The changes in this file effectively integrate the new
companySignatureCollectionNavigationinto the existing navigation structure. The addition is consistent with the established patterns and enhances the portal's functionality for company-related operations.Key points:
- New import added correctly.
- Navigation item placed logically within the hierarchy.
These changes align well with NextJS best practices and maintain type safety with TypeScript.
Also applies to: 51-51
51-51: LGTM: New navigation item added correctly.The
companySignatureCollectionNavigationitem is appropriately placed within theMAIN_NAVIGATIONstructure, maintaining consistency with the existing navigation hierarchy.To ensure the new navigation item is properly implemented, please run the following verification script:
✅ Verification successful
: The
companySignatureCollectionNavigationitem is correctly implemented and properly exported in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of companySignatureCollectionNavigation # Test: Check if the companySignatureCollectionNavigation is exported from the correct module rg --type typescript -A 5 'export.*companySignatureCollectionNavigation' apps/service-portal/signature-collection # Test: Verify the structure of companySignatureCollectionNavigation ast-grep --lang typescript --pattern $'const companySignatureCollectionNavigation: PortalNavigationItem = { $$$ }'Length of output: 1076
libs/service-portal/information/src/lib/navigation.ts (4)
Line range hint
58-70: LGTM: Parliamentary navigation item added correctlyThe new navigation item for parliamentary lists is well-structured and follows the existing patterns in the file. It demonstrates good practices such as:
- Using TypeScript for type safety
- Importing paths from a separate module (
SignatureCollectionPaths) for better modularity- Using the
mobject for labels, which supports internationalizationThese practices align with the coding guidelines for reusability and TypeScript usage.
Line range hint
70-82: LGTM: Presidential navigation item added consistentlyThe presidential lists navigation item is implemented consistently with the parliamentary item. It maintains:
- Proper TypeScript usage
- Modular path imports from
SignatureCollectionPaths- Internationalization support via the
mobjectThis consistency in structure and best practices contributes to the overall maintainability of the codebase.
Line range hint
82-98: LGTM: General Petitions navigation item added with appropriate structureThe general petitions navigation item is well-implemented and extends the pattern established by the previous items. Notable points:
- Consistent use of TypeScript for type safety
- Proper import of paths from
PetitionPaths- Continued use of the
mobject for internationalization- Appropriate addition of a second child item to accommodate both general and owned petitions
This implementation demonstrates good adaptability of the existing structure to meet specific navigation requirements while maintaining code consistency.
Line range hint
58-98: Overall: Excellent implementation of new navigation itemsThe additions to the
informationNavigationobject are well-implemented and consistent. They enhance the navigation structure while adhering to best practices:
- Reusability: The modular structure and use of imported paths (
SignatureCollectionPaths,PetitionPaths) promote reusability across different NextJS apps.- TypeScript usage: Proper use of TypeScript for type safety throughout the additions.
- Internationalization: Consistent use of the
mobject for labels supports multi-language capabilities.- Consistency: The new items follow the established pattern in the file, maintaining code consistency.
These changes align well with the coding guidelines for the
libsdirectory and contribute to a maintainable and scalable codebase.libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (3)
14-14: LGTM: Import statement addition is appropriate.The addition of
useLocationfrom 'react-router-dom' is consistent with the changes in navigation logic and adheres to the guideline of reusability across different NextJS apps.
40-41: LGTM: Prop type definition enhances type safety.The explicit definition of
currentCollection: SignatureCollectionimproves type safety and adheres to the TypeScript usage guideline. This change enhances the component's reusability and maintainability across different NextJS apps.
Line range hint
1-238: Overall changes adhere to coding guidelines and improve component functionality.The modifications to this file, including the addition of
useLocation, explicit prop type definition, and improved navigation logic, are consistent with the coding guidelines forlibs/**/*files. These changes enhance the component's reusability, type safety, and flexibility across different NextJS apps.The updates do not introduce any issues with tree-shaking or bundling practices. The component remains well-structured and maintainable.
libs/service-portal/signature-collection/src/module.tsx (2)
Line range hint
23-83: Validate Access Control and ScopesAll routes are enabled based on
userInfo.scopes.includes(ApiScope.signatureCollection). Ensure that this scope check correctly reflects the intended access control, especially with the introduction of company-specific routes.Please confirm that users without the
signatureCollectionscope cannot access these routes.
67-83: Check for Effective Tree-Shaking and BundlingAdding new routes and components can impact the application's bundle size if not handled properly.
Please verify that the components imported (e.g.,
SignatureListsParliamentary,ViewListParliamentary) are properly optimized for tree-shaking. Ensure that unused code is eliminated during the build process to maintain optimal performance.Run the following script to check for any large imports that might affect bundling:
…om/island-is/island.is into signature-collections-companyroute
albinagu
added
the
deprecated:automerge
* draft * chore: nx format:write update dirty files * fix: company path * tweaks * p * view list company path * chore: nx format:write update dirty files * nav tweak --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: Ásdís Erna Guðmundsdóttir <disa.erna@gmail.com>
* fix(service-portal): company route signature collections (#16382) * draft * chore: nx format:write update dirty files * fix: company path * tweaks * p * view list company path * chore: nx format:write update dirty files * nav tweak --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: Ásdís Erna Guðmundsdóttir <disa.erna@gmail.com> * fix(service-portal): sp list submitted (#16383) * fix(service-portal): sp list submitted * tweak * chore: nx format:write update dirty files --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: andes-it <builders@andes.is> * fix(portals-admin): adding report date (#16384) * fix(portals-admin): adding report date * chore: nx format:write update dirty files * tweak * tweak supervisors info * chore: nx format:write update dirty files * tweak * message - company card * tweak * t * t * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collection): tweaks 15.10 (#16402) * tweaka * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collection): Tweaks for parliamentary collection (#16407) * Tweaks for parliamentary collection * revertt' * Update libs/api/domains/signature-collection/src/lib/signatureCollection.service.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * managers should see other managers --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collection): Fix paper signatures for candidacy (#16411) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(signature-collections): ongoing updates (#16409) * tweaks * tweak - create list * copylink * chore: nx format:write update dirty files --------- Co-authored-by: andes-it <builders@andes.is> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --------- Co-authored-by: albinagu <47886428+albinagu@users.noreply.github.com> Co-authored-by: andes-it <builders@andes.is> Co-authored-by: Ásdís Erna Guðmundsdóttir <disa.erna@gmail.com> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: juni-haukur <158475136+juni-haukur@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation