-
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
fix(portals-admin): adding report date #16384
Conversation
WalkthroughThe changes in this pull request involve several updates across various components related to the signature collection feature. Notably, a date display feature is added to the Changes
Possibly related PRs
Suggested reviewers
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
|
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 and nitpick comments (1)
libs/portals/admin/signature-collection/src/screens-parliamentary/DownloadReports/MyPdfDocument/index.tsx (1)
34-37
: LGTM: Date display added successfullyThe new date display section is well-integrated and uses the
format
function correctly. It maintains the component's reusability and adheres to TypeScript usage.Consider extracting the date format string to a constant for easier maintenance:
const DATE_FORMAT = 'dd.MM.yyyy HH:mm'; // ... <Text style={styles.text}> {format(new Date(), DATE_FORMAT)} </Text>This change would improve maintainability if the date format needs to be updated in the future.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- libs/portals/admin/signature-collection/src/screens-parliamentary/DownloadReports/MyPdfDocument/index.tsx (2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/portals/admin/signature-collection/src/screens-parliamentary/DownloadReports/MyPdfDocument/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 (2)
libs/portals/admin/signature-collection/src/screens-parliamentary/DownloadReports/MyPdfDocument/index.tsx (2)
13-13
: LGTM: Efficient import for date formattingThe import of the
format
function fromdate-fns/format
is a good practice. It aligns with effective tree-shaking by importing only the necessary function, which can help reduce bundle size.
Line range hint
1-37
: Adherence to coding guidelines confirmedThe changes in this file comply with the coding guidelines for
libs/**/*
:
- The component remains reusable across different NextJS apps.
- TypeScript is used for defining props (SignatureCollectionAreaSummaryReport).
- The new import (
format
fromdate-fns/format
) supports effective tree-shaking and bundling practices.
Datadog ReportAll test runs ✅ 70 Total Test Services: 0 Failed, 67 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
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: 1
🧹 Outside diff range and nitpick comments (7)
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/index.tsx (1)
Line range hint
1-58
: Ensure adherence to library coding guidelinesAs this component is located in a library folder, please verify the following:
Reusability: The component seems tightly coupled to the signature collection feature. Consider if it can be made more generic for potential reuse across different NextJS apps.
TypeScript usage: While the file uses TypeScript, there are no explicit prop type definitions or exported types. Consider defining an interface for the component's props and exporting any relevant types.
Tree-shaking: The component imports several UI components from '@island.is/island-ui/core'. Ensure that only the necessary components are imported to facilitate effective tree-shaking.
Consider refactoring the component to improve its reusability and TypeScript usage:
import React from 'react'; import { Box, Stack, Text } from '@island.is/island-ui/core'; import { useLocale, useNamespaces } from '@island.is/localization'; import { m } from '../../../../lib/messages'; import { useParams } from 'react-router-dom'; import { useGetSignatureList } from '../../../../hooks'; import format from 'date-fns/format'; import Signees from './Signees'; interface ViewListProps { // Add any props if needed } export interface ListInfo { title: string; startTime: string; endTime: string; numberOfSignatures: number; collectors?: Array<{ name: string }>; } export const ViewList: React.FC<ViewListProps> = () => { // Component logic... }; export default ViewList;This refactoring adds explicit type definitions and exports the
ListInfo
interface, which could be useful for other components or tests.libs/service-portal/signature-collection/src/screens/shared/SigneeView/index.tsx (1)
Line range hint
1-138
: Consider enhancing type definitions and exports for better reusabilityThe component generally adheres to the coding guidelines for library files. However, to further improve reusability and type safety, consider the following suggestions:
- Define and export an interface for the component's props.
- Use more specific types instead of any where possible.
- Consider exporting any shared types or interfaces that might be useful for consumers of this component.
Example:
export interface SigneeViewProps { currentCollection: SignatureCollection; } const SigneeView: React.FC<SigneeViewProps> = ({ currentCollection }) => { // ... component logic ... } export default SigneeView;These changes would enhance the component's reusability across different NextJS apps and improve type safety.
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/PaperSignees.tsx (2)
74-80
: Improved error handling and user feedback. Consider type safety enhancements.The changes improve the error handling and user feedback mechanism by checking the success status of the operation and displaying appropriate toast messages. Good job on calling
refetchSignees
only on success to prevent unnecessary refetches.To further enhance the code:
Consider adding type safety for the
res
object:onCompleted: (res: { success: boolean }) => { if (res?.success) { // ... } }The error case in
onCompleted
might be redundant with theonError
callback. You could simplify it to:onCompleted: (res: { success: boolean }) => { if (res?.success) { toast.success(formatMessage(m.paperSigneeSuccess)) refetchSignees() } }, onError: () => { toast.error(formatMessage(m.paperSigneeError)) },
Line range hint
1-1
: Consider exporting types for better reusabilityThe component adheres well to the coding guidelines for
libs
directory files, using TypeScript and shared libraries. To further enhance reusability across different NextJS apps, consider exporting the prop types of thePaperSignees
component.Add the following at the beginning of the file:
export interface PaperSigneesProps { listId: string; refetchSignees: () => void; }Then update the component definition:
export const PaperSignees: React.FC<PaperSigneesProps> = ({ listId, refetchSignees, }) => { // ... }This change will make it easier for other parts of the application to import and use the
PaperSignees
component with proper type checking.libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (1)
190-194
: LGTM! Consider consistent formatting for readability.The change improves internationalization by using a localized message for the tooltip text, which enhances user understanding and aligns with the coding guidelines for reusability across different NextJS apps.
For consistency with other similar Tooltip components in this file (e.g., line 84), consider formatting the Tooltip props on separate lines:
<Tooltip placement="right" text={formatMessage(m.supervisorsInfo)} color="blue400" />libs/service-portal/signature-collection/src/lib/messages.ts (2)
176-179
: Improved clarity and user guidance for access description.The updated
actorNoAccessDescription
message provides more specific information about who can create a signature collection and what actions users can take. This change enhances user understanding and guidance.Consider adding a link to the location where users can create a signature collection. This could be done by replacing "hér" with a more descriptive text and wrapping it in markdown link syntax. For example:
- 'Eingöngu kennitölur sem hafa skráðan listabókstaf geta stofnað meðmælendasöfnun. Ef þú átt listabókstaf geturðu stofnað meðmælendasöfnun hér.', + 'Eingöngu kennitölur sem hafa skráðan listabókstaf geta stofnað meðmælendasöfnun. Ef þú átt listabókstaf geturðu [stofnað meðmælendasöfnun á þessari síðu](/path-to-create-collection).',Replace
/path-to-create-collection
with the actual path where users can create a signature collection.
358-363
: Clear explanation of supervisor access rights added.The new
supervisorsInfo
message provides valuable information about the role and permissions of supervisors in the signature collection process. This addition enhances transparency and user understanding.For consistency with other messages in the file, consider removing the trailing space in the
defaultMessage
. Apply this minor change:defaultMessage: - 'Umsjónaraðilar hafa aðgang að öllum upplýsingum safnananna en geta ekki eytt söfnununum. ', + 'Umsjónaraðilar hafa aðgang að öllum upplýsingum safnananna en geta ekki eytt söfnununum.', description: '',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
- libs/service-portal/core/src/lib/messages.ts (1 hunks)
- libs/service-portal/signature-collection/src/lib/messages.ts (2 hunks)
- libs/service-portal/signature-collection/src/lib/navigation.ts (1 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/PaperSignees.tsx (1 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/index.tsx (1 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (1 hunks)
- libs/service-portal/signature-collection/src/screens/shared/SigneeView/index.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
libs/service-portal/core/src/lib/messages.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/messages.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/screens/Parliamentary/OwnerView/ViewList/Signees/PaperSignees.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/ViewList/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."
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."
libs/service-portal/signature-collection/src/screens/shared/SigneeView/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 (3)
libs/service-portal/signature-collection/src/lib/navigation.ts (1)
11-11
: LGTM! Verify consistency of the new message key.The change to use
m.signatureCollectionParliamentaryListsCompany
for the description makes it more specific to company signature collections, which is a good improvement for clarity.Let's verify that this new message key is defined and used consistently:
This script will help ensure that the new message key is properly defined and that we haven't missed updating it in any other navigation-related files.
libs/service-portal/signature-collection/src/screens/shared/SigneeView/index.tsx (1)
110-110
: Improved localization for AlertMessageThis change enhances the localization support for the alert message. By using
formatMessage(m.actorNoAccessDescription)
instead of the staticdefaultMessage
, the component now dynamically formats the message based on the current locale. This is consistent with how other messages in the component are handled and aligns with best practices for internationalization.Good job on improving the user experience for different locales!
libs/service-portal/core/src/lib/messages.ts (1)
568-572
: New message for parliamentary lists signature collection added correctly.The new message
signatureCollectionParliamentaryListsCompany
has been added with the correct format and follows the existing pattern. Theid
anddefaultMessage
are appropriate for the context.Let's check if there are any related messages that might need updates:
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/index.tsx
Show resolved
Hide resolved
* 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 <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.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 <[email protected]> Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]> * 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 <[email protected]> * 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --------- Co-authored-by: albinagu <[email protected]> Co-authored-by: andes-it <[email protected]> Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> Co-authored-by: juni-haukur <[email protected]> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Checklist:
Summary by CodeRabbit