Skip to content
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

feat(web): grant slice for articles #17262

Draft
wants to merge 29 commits into
base: main
Choose a base branch
from
Draft

feat(web): grant slice for articles #17262

wants to merge 29 commits into from

Conversation

thorkellmani
Copy link
Member

@thorkellmani thorkellmani commented Dec 16, 2024

What

  • New web component, GrantCardsList
  • Add as slice to Article

Why

  • Allows organizations to automatically display grants belonging to a specific fund easily.

Screenshots / Gifs

image

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Formatting passes locally with my changes
  • I have rebased against main before asking for a review

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a new GrantCardsList component for displaying grant cards.
    • Added a new entry type ID, grantCardsList, to the editable entry types.
    • Enhanced GraphQL schema with a new fragment for grant-related data.
    • Added a new optional field funds to the grants input for filtering by funds.
    • Integrated GrantCardsList into the rendering logic for improved user experience.
  • Bug Fixes

    • Improved error handling in the getSingleVacancy method.
  • Documentation

    • Updated type definitions and interfaces for better clarity and functionality.
  • Style

    • Updated the border color of the InfoCard component for improved visual consistency.

Copy link
Contributor

coderabbitai bot commented Dec 16, 2024

Walkthrough

This pull request introduces a comprehensive enhancement to the grant cards functionality across multiple files in the project. The changes include adding a new GrantCardsList component, updating GraphQL resolvers and models, modifying the CMS service to support fund-based filtering, and integrating the new component into various parts of the application. The modifications span from frontend components to backend services, ensuring a cohesive implementation of the grant cards feature.

Changes

File Change Summary
apps/tools/contentful-role-permissions/constants/index.ts Added grantCardsList to DEFAULT_EDITABLE_ENTRY_TYPE_IDS
apps/web/components/GrantCardsList/GrantCardsList.tsx New React component for displaying grant cards
apps/web/components/GrantCardsList/index.ts Added dynamic import for GrantCardsList component
apps/web/components/GrantCardsList/types.ts Introduced TranslationKeys type for grant card translations
apps/web/screens/Home/Home.tsx Updated imports and component structure
apps/web/screens/queries/fragments.ts Added GrantCardsListFields GraphQL fragment
apps/web/utils/richText.tsx Integrated GrantCardsList into rendering components
libs/cms/src/lib/cms.elasticsearch.service.ts Updated getGrants method to include funds parameter
libs/cms/src/lib/cms.module.ts Added GrantCardsListResolver to providers and imports
libs/cms/src/lib/cms.resolver.ts Introduced GrantCardsListResolver with new methods
libs/cms/src/lib/dto/getGrants.input.ts Added funds field to GetGrantsInput
libs/cms/src/lib/models/grantCardsList.model.ts New model for grant cards
libs/cms/src/lib/unions/slice.union.ts Added GrantCardsList to slice types
libs/island-ui/core/src/lib/InfoCardGrid/InfoCard.tsx Minor styling update

Possibly related PRs

Suggested Labels

automerge

Suggested Reviewers

  • RunarVestmann
  • disaerna

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Dec 16, 2024

Codecov Report

Attention: Patch coverage is 18.88112% with 116 lines in your changes missing coverage. Please review.

Project coverage is 35.71%. Comparing base (7fe9be5) to head (56eed5a).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...s/web/components/GrantCardsList/GrantCardsList.tsx 0.00% 89 Missing ⚠️
libs/cms/src/lib/cms.resolver.ts 47.36% 10 Missing ⚠️
libs/cms/src/lib/models/grantCardsList.model.ts 66.66% 8 Missing ⚠️
libs/cms/src/lib/cms.elasticsearch.service.ts 0.00% 2 Missing ⚠️
...ibs/cms/src/lib/search/importers/grants.service.ts 0.00% 2 Missing ⚠️
libs/cms/src/lib/unions/slice.union.ts 33.33% 2 Missing ⚠️
apps/web/components/GrantCardsList/index.ts 0.00% 1 Missing ⚠️
apps/web/utils/richText.tsx 0.00% 1 Missing ⚠️
libs/cms/src/lib/dto/getGrants.input.ts 50.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17262      +/-   ##
==========================================
- Coverage   35.72%   35.71%   -0.02%     
==========================================
  Files        6946     6949       +3     
  Lines      148584   148727     +143     
  Branches    42415    42482      +67     
==========================================
+ Hits        53085    53114      +29     
- Misses      95499    95613     +114     
Flag Coverage Δ
air-discount-scheme-web 0.00% <ø> (ø)
api 3.33% <ø> (ø)
api-domains-auth-admin 48.49% <ø> (ø)
api-domains-communications 39.47% <49.01%> (+0.04%) ⬆️
application-api-files 61.87% <ø> (ø)
application-core 75.72% <ø> (ø)
application-system-api 38.81% <51.92%> (+0.01%) ⬆️
application-template-api-modules 27.86% <45.09%> (+0.05%) ⬆️
application-templates-accident-notification 28.80% <ø> (ø)
application-templates-car-recycling 3.12% <ø> (ø)
application-templates-criminal-record 25.74% <ø> (ø)
application-templates-driving-license 18.15% <ø> (ø)
application-templates-estate 13.69% <ø> (ø)
application-templates-example-payment 24.69% <ø> (ø)
application-templates-financial-aid 14.45% <ø> (ø)
application-templates-general-petition 23.12% <ø> (ø)
application-templates-inheritance-report 6.59% <ø> (ø)
application-templates-marriage-conditions 15.18% <ø> (ø)
application-templates-mortgage-certificate 43.62% <ø> (ø)
application-templates-parental-leave 30.02% <ø> (+0.09%) ⬆️
application-types 6.51% <ø> (ø)
application-ui-components 1.22% <ø> (ø)
application-ui-shell 22.31% <ø> (ø)
clients-charge-fjs-v2 28.35% <ø> (ø)
cms 0.39% <0.00%> (-0.01%) ⬇️
cms-translations 38.79% <45.09%> (+0.03%) ⬆️
contentful-apps 4.72% <ø> (ø)
financial-aid-backend 51.40% <ø> (ø)
financial-aid-shared 17.88% <ø> (ø)
island-ui-core 30.40% <ø> (ø)
judicial-system-api 20.21% <ø> (ø)
judicial-system-backend 55.91% <51.92%> (-0.01%) ⬇️
judicial-system-web 27.77% <ø> (ø)
portals-admin-regulations-admin 1.80% <ø> (ø)
portals-core 19.61% <ø> (ø)
services-user-notification 46.57% <51.92%> (+0.02%) ⬆️
shared-components 29.64% <ø> (ø)
shared-form-fields 33.36% <ø> (ø)
web 2.39% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...web/components/TableOfContents/TableOfContents.tsx 0.00% <ø> (ø)
apps/web/screens/Home/Home.tsx 0.00% <ø> (ø)
apps/web/screens/queries/fragments.ts 0.00% <ø> (ø)
libs/cms/src/lib/cms.module.ts 100.00% <ø> (ø)
...s/island-ui/core/src/lib/InfoCardGrid/InfoCard.tsx 6.25% <ø> (ø)
apps/web/components/GrantCardsList/index.ts 0.00% <0.00%> (ø)
apps/web/utils/richText.tsx 0.00% <0.00%> (ø)
libs/cms/src/lib/dto/getGrants.input.ts 61.90% <50.00%> (-1.26%) ⬇️
libs/cms/src/lib/cms.elasticsearch.service.ts 2.86% <0.00%> (-0.03%) ⬇️
...ibs/cms/src/lib/search/importers/grants.service.ts 11.11% <0.00%> (-0.37%) ⬇️
... and 4 more

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fe9be5...56eed5a. Read the comment docs.

@datadog-island-is
Copy link

datadog-island-is bot commented Dec 16, 2024

Datadog Report

All test runs a2fbecd 🔗

10 Total Test Services: 0 Failed, 9 Passed
🔻 Test Sessions change in coverage: 4 decreased, 2 increased, 165 no change

Test Services
This report shows up to 10 services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Code Coverage Change Test Service View
air-discount-scheme-web 0 0 0 2 0 6.71s 1 no change Link
api 0 0 0 4 0 2.44s 1 no change Link
api-domains-auth-admin 0 0 0 18 0 10.22s 1 no change Link
api-domains-communications 0 0 0 5 0 33.6s 1 increased (+0.03%) Link
api-domains-license-service 0 0 0 0 0 486.11ms 1 no change Link
application-api-files 0 0 0 2 0 4.37s 1 decreased (-0.03%) Link
application-core 0 0 0 97 0 15.65s 1 no change Link
application-system-api 0 0 0 46 0 2m 16.87s 1 no change Link
application-template-api-modules 0 0 0 118 0 2m 11.79s 1 increased (+0.03%) Link
application-templates-accident-notification 0 0 0 148 0 15.15s 1 no change Link

🔻 Code Coverage Decreases vs Default Branch (4)

  • application-api-files - jest 65.9% (-0.03%) - Details
  • services-user-notification - jest 68.89% (-0.03%) - Details
  • judicial-system-backend - jest 59.52% (-0.02%) - Details
  • web - jest 3.23% (-0.01%) - Details

@thorkellmani thorkellmani added the deploy-feature Deploys features to dev label Dec 16, 2024
Copy link
Contributor

Affected services are: air-discount-scheme-api,api,application-system-api,financial-aid-api,financial-aid-backend,financial-aid-open-api,judicial-system-api,judicial-system-backend,services-auth-personal-representative,services-search-indexer,services-user-notification,air-discount-scheme-web,consultation-portal,contentful-apps,financial-aid-web-osk,financial-aid-web-veita,judicial-system-web,skilavottord-web,web,application-system-form,island-ui-storybook,portals-admin,service-portal,system-e2e,
Feature deployment of your services will begin shortly. Your feature will be accessible here:
https://featgrant-slice-api-catalogue.dev01.devland.is/api
https://featgrant-slice-application-callback-xrd.internal.dev01.devland.is/application-payment
https://featgrant-slice-application-callback-xrd.internal.dev01.devland.is/applications
https://featgrant-slice-application-payment-callback-xrd.internal.dev01.devland.is/application-payment
https://featgrant-slice-application-payment-callback-xrd.internal.dev01.devland.is/applications
https://featgrant-slice-beta.dev01.devland.is/
https://featgrant-slice-beta.dev01.devland.is/api
https://featgrant-slice-beta.dev01.devland.is/app/skilavottord/
https://featgrant-slice-beta.dev01.devland.is/bff
https://featgrant-slice-beta.dev01.devland.is/minarsidur
https://featgrant-slice-beta.dev01.devland.is/samradsgatt
https://featgrant-slice-beta.dev01.devland.is/stjornbord
https://featgrant-slice-beta.dev01.devland.is/stjornbord/bff
https://featgrant-slice-beta.dev01.devland.is/umsoknir
https://featgrant-slice-loftbru-cf.dev01.devland.is/
https://featgrant-slice-loftbru-cf.dev01.devland.is/api/graphql
https://featgrant-slice-loftbru.dev01.devland.is/
https://featgrant-slice-loftbru.dev01.devland.is/api/graphql
https://featgrant-slice-ui.dev01.devland.is/

Deployed services: application-system-api,application-system-form,service-portal,portals-admin,consultation-portal,api,web,skilavottord-web,island-ui-storybook,air-discount-scheme-web,air-discount-scheme-api,application-system-api-worker,services-bff-portals-admin,services-bff-portals-my-pages.
Excluded services: search-indexer-service,user-notification,user-notification-worker,user-notification-cleanup-worker,contentful-apps

@thorkellmani thorkellmani marked this pull request as ready for review December 18, 2024 10:47
@thorkellmani thorkellmani requested review from a team as code owners December 18, 2024 10:47
Copy link
Member

@disaerna disaerna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor date format comment otherwise 👍🏼

apps/web/components/GrantCardsList/GrantCardsList.tsx Outdated Show resolved Hide resolved
apps/web/components/GrantCardsList/GrantCardsList.tsx Outdated Show resolved Hide resolved
@disaerna disaerna removed the request for review from Tryggvig December 18, 2024 12:55
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🔭 Outside diff range comments (1)
apps/web/screens/queries/fragments.ts (1)

Line range hint 1-1200: Remove duplicate EmbedFields fragment.

The EmbedFields fragment appears twice in the file. This could lead to GraphQL validation errors.

Remove one of the duplicate fragments to ensure proper GraphQL schema validation.

🧹 Nitpick comments (9)
apps/web/components/GrantCardsList/GrantCardsList.tsx (4)

1-6: Ensure NextJS/React import usage adheres to best practices.

Even though this is a client-side component, you may consider whether any SSR or SSG approach could enhance performance or SEO. However, for an exclusively dynamic component, depending on user interaction (like a router push), this pattern looks fine.


23-36: Provide a more descriptive error message.

Catching an exception within the date formatter is good, but “Error formatting date” is too generic. Providing the date or a mention of the failing locale may help with debugging.


111-126: Provide a fallback UI for single-item scenario.

Rendering a single ActionCard is a great approach. Consider how it handles situations where the Grant data might be missing essential fields like name. Currently, the card would render with an empty heading. Handling empty states explicitly can promote consistent UX.


128-184: Validate date fields before formatting.

When mapping over grants, deeper validation before calling format(...) may prevent runtime errors if grant.dateFrom or grant.dateTo isn’t a valid ISO string. A quick check for valid dates or try/catch around format calls would be safer.

libs/cms/src/lib/models/grantCardsList.model.ts (2)

12-17: Consider adding descriptions to enum values

The CardSorting enum would benefit from GraphQL descriptions to improve API documentation.

 enum CardSorting {
+  @Field({ description: 'Sort grants alphabetically by title' })
   ALPHABETICAL,
+  @Field({ description: 'Sort grants by most recently updated first' })
   MOST_RECENTLY_UPDATED_FIRST,
 }

37-55: LGTM with minor enhancement suggestion

The mapping function is well-structured and handles nullable fields appropriately. Consider adding input validation for required fields.

 export const mapGrantCardsList = ({
   fields,
   sys,
 }: IGrantCardsList): SystemMetadata<GrantCardsList> => {
+  if (!fields.grantCardListTitle) {
+    throw new Error('Grant card list title is required');
+  }
+
   return {
     // ... rest of the implementation
   }
 }
libs/cms/src/lib/search/importers/grants.service.ts (1)

102-107: Consider extracting tag creation logic.

The tag creation logic is repeated across different tag types. Consider extracting this into a helper function to improve maintainability and reduce duplication.

+const createTag = (type: string, key: string, value?: string) => ({
+  type,
+  key,
+  value,
+});

-tags.push({
-  type: 'organization',
-  key: mapped.fund.parentOrganization.slug,
-  value: mapped.fund.parentOrganization.title,
-});
+tags.push(createTag(
+  'organization',
+  mapped.fund.parentOrganization.slug,
+  mapped.fund.parentOrganization.title
+));
libs/cms/src/lib/cms.resolver.ts (2)

863-875: Optimize the input validation condition.

The condition can be simplified by removing the redundant optional chaining operator.

-    if (!input || input?.size === 0) {
+    if (!input || input.size === 0) {

879-879: Fix typo in variable name.

There's a typo in the variable name 'respones'.

-      const respones = await this.cmsContentfulService.getNamespace(
+      const response = await this.cmsContentfulService.getNamespace(
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab48b56 and 7d6dc62.

⛔ Files ignored due to path filters (1)
  • libs/cms/src/lib/generated/contentfulTypes.d.ts is excluded by !**/generated/**
📒 Files selected for processing (15)
  • apps/tools/contentful-role-permissions/constants/index.ts (1 hunks)
  • apps/web/components/GrantCardsList/GrantCardsList.tsx (1 hunks)
  • apps/web/components/GrantCardsList/index.ts (1 hunks)
  • apps/web/components/GrantCardsList/types.ts (1 hunks)
  • apps/web/screens/Home/Home.tsx (1 hunks)
  • apps/web/screens/queries/fragments.ts (2 hunks)
  • apps/web/utils/richText.tsx (3 hunks)
  • libs/cms/src/lib/cms.elasticsearch.service.ts (2 hunks)
  • libs/cms/src/lib/cms.module.ts (2 hunks)
  • libs/cms/src/lib/cms.resolver.ts (2 hunks)
  • libs/cms/src/lib/dto/getGrants.input.ts (1 hunks)
  • libs/cms/src/lib/models/grantCardsList.model.ts (1 hunks)
  • libs/cms/src/lib/search/importers/grants.service.ts (1 hunks)
  • libs/cms/src/lib/unions/slice.union.ts (5 hunks)
  • libs/island-ui/core/src/lib/InfoCardGrid/InfoCard.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • apps/web/components/GrantCardsList/index.ts
🧰 Additional context used
📓 Path-based instructions (14)
libs/island-ui/core/src/lib/InfoCardGrid/InfoCard.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."
apps/tools/contentful-role-permissions/constants/index.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/cms/src/lib/dto/getGrants.input.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."
apps/web/components/GrantCardsList/types.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/cms/src/lib/cms.module.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/cms/src/lib/search/importers/grants.service.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."
apps/web/screens/queries/fragments.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."
apps/web/utils/richText.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/cms/src/lib/models/grantCardsList.model.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/cms/src/lib/cms.elasticsearch.service.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."
apps/web/components/GrantCardsList/GrantCardsList.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."
apps/web/screens/Home/Home.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/cms/src/lib/cms.resolver.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/cms/src/lib/unions/slice.union.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."
🔇 Additional comments (18)
libs/cms/src/lib/dto/getGrants.input.ts (1)

47-50: Optional field introduced—validate upstream usage.

The newly added 'funds' field allows multiple string entries. Ensure the resolver and service logic handle empty arrays or undefined values gracefully.

✅ Verification successful

Let me analyze the code further to verify the handling of the optional funds field.


Based on the previous outputs, I can now generate a final response since we have seen:

  1. The elasticsearch service properly handles the optional funds field with a conditional check (if (funds)) before using it in the query
  2. The resolver simply passes the input to the elasticsearch service
  3. The field is properly decorated with @IsOptional() and @IsArray() validations

Optional field is properly handled

The newly added funds field is correctly implemented with proper validation decorators and the service layer handles the optional nature of the field appropriately. The elasticsearch query is constructed conditionally when the field is present, ensuring graceful handling of empty arrays or undefined values.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Searching the codebase for all references to `funds` usage in the relevant grants resolver/service.
rg -A 4 "funds" 

Length of output: 21577


Script:

#!/bin/bash
# Let's check the implementation of the grants resolver and service
ast-grep --pattern 'class $_ {
  $$$
  getGrants($$$) {
    $$$
  }
  $$$
}'

Length of output: 78


Script:

#!/bin/bash
# Let's check the elasticsearch service implementation for handling funds
rg -A 10 "getGrants.*Input" --type ts

Length of output: 746

apps/tools/contentful-role-permissions/constants/index.ts (1)

38-38: Confirm alignment with Contentful entry type.

The grantCardsList entry is now editable. Verify that there’s a corresponding content type in Contentful and that necessary permissions are in place.

✅ Verification successful

Content type and permissions are properly configured

Based on the generated Contentful type definitions, the grantCardsList content type is properly defined in Contentful with all necessary fields including title, display options, funds relationship, max number of cards, and sorting preferences. The type is correctly integrated into the codebase with proper model definitions and resolvers.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Attempt to locate references to `grantCardsList` in the code, ensuring usage is consistent.
rg -A 3 "grantCardsList"

Length of output: 3115


Script:

#!/bin/bash
# Let's check the contentful type definition for grantCardsList
ast-grep --pattern 'interface IGrantCardsList {
  $$$
}'

Length of output: 58


Script:

#!/bin/bash
# Let's check the contentful type definition in the generated types file
cat libs/cms/src/lib/generated/contentfulTypes.d.ts | grep -A 20 "export interface IGrantCardsList"

Length of output: 915

libs/island-ui/core/src/lib/InfoCardGrid/InfoCard.tsx (1)

57-57: Verify design system color token usage

The change from "white" to "blue200" affects all instances of InfoCard. Ensure this change is intentional and aligns with the design system.

✅ Verification successful

Change to blue200 border color aligns with design system

The change from "white" to "blue200" is consistent with the design system's color tokens. The blue200 color is widely used across the codebase for borders, particularly in UI components that require subtle emphasis or structure. This color token is defined in the theme system and is used consistently for similar UI patterns.

Key findings from the codebase:

  • blue200 is a standard border color token defined in the theme system
  • It's commonly used for borders in card-like components and interactive elements
  • The color is part of the established design system, appearing in multiple UI libraries and applications
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other components using InfoCard to assess impact
rg -l "InfoCard" --type tsx

# Check for other uses of blue200 color token
rg "blue200" --type tsx

Length of output: 107


Script:

#!/bin/bash
# Search for other components using InfoCard to assess impact
rg -l "InfoCard"

# Check for other uses of blue200 color token
rg "blue200"

# Check design system color tokens
fd "colors" --type f

Length of output: 41680

libs/cms/src/lib/cms.module.ts (1)

12-12: Verify resolver dependencies and initialization order

The GrantCardsListResolver is correctly added to both imports and providers. However, verify that there are no circular dependencies with other resolvers.

Also applies to: 45-45

✅ Verification successful

Let me gather more information about potential circular dependencies.


Let me check for any potential circular dependencies in the resolver implementation.


Based on the analysis of the codebase, I can now provide the final response:

No circular dependencies or initialization order issues found

The GrantCardsListResolver is properly implemented with:

  • Clean dependency injection of required services (cmsElasticsearchService and cmsContentfulService)
  • No circular imports or dependencies detected
  • Proper resolver field implementations with appropriate error handling
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for GrantCardsListResolver dependencies
rg "GrantCardsListResolver" --type ts

# Check for circular imports
ast-grep --pattern 'import { $_ } from "./cms.resolver"'

Length of output: 283


Script:

#!/bin/bash
# Check the content of cms.resolver.ts to see its dependencies
cat libs/cms/src/lib/cms.resolver.ts

# Look for any imports in the resolver file
ast-grep --pattern 'import { $_ } from "$_"' libs/cms/src/lib/cms.resolver.ts

Length of output: 33272


Script:

#!/bin/bash
# Check for GrantCardsListResolver implementation and its dependencies
ast-grep --pattern 'class GrantCardsListResolver {
  $$$
}'

# Check for any imports of GrantCardsListResolver in other files
rg "import.*GrantCardsListResolver" --type ts

Length of output: 2302

apps/web/screens/Home/Home.tsx (1)

1-32: LGTM! Clean and well-organized imports.

The import statements are properly organized and follow TypeScript best practices with explicit type imports.

libs/cms/src/lib/search/importers/grants.service.ts (1)

94-100: LGTM! Well-structured fund tagging implementation.

The new fund tagging logic is properly implemented with type safety and follows the existing tag structure pattern.

apps/web/utils/richText.tsx (2)

69-69: LGTM! Clean integration of GrantCardsList.

The import statements and type definitions are properly organized and follow the established pattern.

Also applies to: 95-95


295-297: LGTM! Consistent component mapping implementation.

The GrantCardsList component is properly integrated into the defaultRenderComponent object, following the existing pattern for slice rendering.

libs/cms/src/lib/unions/slice.union.ts (4)

51-51: LGTM: Import declarations follow the established pattern.

The imports are correctly organized, following the same pattern as other slice types in the file.

Also applies to: 149-152


199-199: LGTM: SliceTypes union is properly extended.

The IGrantCardsList type is correctly added to the union type.


253-253: LGTM: SliceUnion type array is properly updated.

The GrantCardsList type is correctly added to the types array.


351-352: LGTM: mapSliceUnion function is properly extended.

The new case for 'grantCardsList' is correctly added with proper type casting.

apps/web/screens/queries/fragments.ts (1)

795-851: LGTM: GraphQL fragment is well-structured and comprehensive.

The fragment includes all necessary fields for grants, funds, and organizations, following GraphQL best practices.

libs/cms/src/lib/cms.elasticsearch.service.ts (3)

619-619: LGTM: Method signature properly updated.

The funds parameter is correctly added to the method signature.


709-719: LGTM: Organization tags query is well-structured.

The nested query for organization tags follows Elasticsearch best practices.


Line range hint 721-738: LGTM: Fund tags query implementation is correct.

The nested query for fund tags follows the same pattern as other tag filters and properly implements the fund filtering functionality.

✅ Verification successful

Let me gather more context about the tag implementation to ensure the fund tag filtering is consistent.


Let me search for more tag filtering implementations to ensure consistency.


Based on the search results, I can see that the tag filtering pattern is consistently implemented across the codebase. The fund tag query follows the same nested query structure as other tag types like 'genericTag', 'organization', 'referencedBy', etc. Each tag filter uses the same pattern of checking both the tag type and key/value in a nested query.

LGTM: Fund tag filtering implementation is consistent with other tag filters

The implementation correctly uses the nested query pattern with 'tags.type': 'fund' which matches the established pattern used for other tag types in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the fund tag type usage in other files
rg -A 2 "tags\.type.*fund" 

Length of output: 247


Script:

#!/bin/bash
# Search for tag type definitions and implementations
ast-grep --pattern 'tags: { type: $_ }'

# Also check for any tag-related interfaces or types
rg -A 5 "interface.*Tag" 
rg -A 5 "type.*Tag"

# Look for other tag filtering implementations
ast-grep --pattern $'nested: {
  path: "tags",
  query: $$$
}'

Length of output: 114270


Script:

#!/bin/bash
# Search for tag type usage in queries
rg -A 5 "'tags\.type'.*:|'tags\.type'.*=>" 

# Look for tag filtering in elasticsearch service
rg -A 10 "nested.*path: 'tags'" libs/cms/src/lib/cms.elasticsearch.service.ts

Length of output: 4856

libs/cms/src/lib/cms.resolver.ts (2)

139-139: LGTM!

The import statement follows the established pattern and is correctly placed with other model imports.


855-861: LGTM!

The resolver class follows the established pattern with:

  • Proper decorator usage
  • Correct dependency injection
  • Consistent cache control configuration

apps/web/components/GrantCardsList/types.ts Show resolved Hide resolved
libs/cms/src/lib/cms.resolver.ts Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (3)
apps/web/screens/Home/Home.tsx (3)

Line range hint 47-51: Consider moving document language setting to an effect or layout component

The document language setting should be handled in a useEffect or moved to a layout component to ensure proper side-effect management:

- if (typeof document === 'object') {
-   document.documentElement.lang = activeLocale
- }
+ useEffect(() => {
+   if (typeof document === 'object') {
+     document.documentElement.lang = activeLocale
+   }
+ }, [activeLocale])

Line range hint 114-117: Remove TypeScript ignore comment and fix the underlying type issue

The @ts-ignore comment should be addressed by properly typing the WatsonChatPanel component:

Consider creating proper types for the WatsonChatPanel configuration:

interface WatsonConfig {
  // Add appropriate types based on the actual config
  [key in Locale]?: WatsonPanelProps
}

Line range hint 121-185: Add error handling and loading states for data fetching

The current implementation lacks error handling for GraphQL queries. Consider implementing proper error boundaries and loading states:

Home.getProps = async ({ apolloClient, locale }) => {
+ try {
    const [
      { data: { getArticleCategories } },
      { data: { getNews: { items: news } } },
      { data: { getFrontpage } }
    ] = await Promise.all([
      // ... existing queries ...
    ])

    return {
      // ... existing return ...
    }
+ } catch (error) {
+   console.error('Failed to fetch home page data:', error)
+   return {
+     news: [],
+     categories: [],
+     page: null,
+     showSearchInHeader: false,
+     locale: locale as Locale,
+   }
+ }
}
🧹 Nitpick comments (5)
libs/cms/src/lib/models/grantCardsList.model.ts (3)

11-16: Add descriptions for the enum values

Consider adding descriptions to the enum registration for better API documentation.

-registerEnumType(CardSorting, { name: 'GrantCardsListSorting' })
+registerEnumType(CardSorting, {
+  name: 'GrantCardsListSorting',
+  description: 'Sorting options for grant cards',
+  valuesMap: {
+    ALPHABETICAL: { description: 'Sort grants alphabetically by title' },
+    MOST_RECENTLY_UPDATED_FIRST: { description: 'Sort grants by their last update timestamp' }
+  }
+})

29-30: Improve type safety for resolvedGrantsList

The resolvedGrantsList field type could be more specific than GetGrantsInput.

-  @CacheField(() => GrantList, { nullable: true })
-  resolvedGrantsList?: GetGrantsInput
+  @CacheField(() => GrantList, { nullable: true })
+  resolvedGrantsList?: Pick<GetGrantsInput, 'lang' | 'funds' | 'size'>

46-49: Enhance locale handling robustness

The current locale handling assumes only 'is-IS' as a special case. Consider using a more maintainable approach.

-      lang:
-        sys.locale === 'is-IS'
-          ? 'is'
-          : (sys.locale as ElasticsearchIndexLocale),
+      lang: mapLocaleToLang(sys.locale),

Add this helper function:

const mapLocaleToLang = (locale: string): ElasticsearchIndexLocale => {
  const localeMap: Record<string, ElasticsearchIndexLocale> = {
    'is-IS': 'is',
    // Add other locale mappings as needed
  };
  return localeMap[locale] ?? (locale as ElasticsearchIndexLocale);
};
apps/web/screens/Home/Home.tsx (2)

4-22: Consider strengthening TypeScript type safety

While the imports are well-organized, consider adding more specific types for the GraphQL queries to improve type safety:

- GetArticleCategoriesQuery,
+ GetArticleCategoriesQuery['getArticleCategories'],
- GetFrontpageQuery,
+ GetFrontpageQuery['getFrontpage'],

Line range hint 35-41: Add JSDoc documentation for the interface

Consider adding JSDoc documentation to improve maintainability and developer experience:

+/**
+ * Props for the Home component
+ * @property {GetArticleCategoriesQuery['getArticleCategories']} categories - Article categories
+ * @property {GetNewsQuery['getNews']['items']} news - News items
+ * @property {GetFrontpageQuery['getFrontpage']} page - Frontpage data
+ * @property {Locale} locale - Current locale
+ */
interface HomeProps {
  categories: GetArticleCategoriesQuery['getArticleCategories']
  news: GetNewsQuery['getNews']['items']
  page: GetFrontpageQuery['getFrontpage']
  locale: Locale
}
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d6dc62 and 6c5db77.

📒 Files selected for processing (2)
  • apps/web/screens/Home/Home.tsx (1 hunks)
  • libs/cms/src/lib/models/grantCardsList.model.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/cms/src/lib/models/grantCardsList.model.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."
apps/web/screens/Home/Home.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."
🔇 Additional comments (4)
libs/cms/src/lib/models/grantCardsList.model.ts (3)

1-10: LGTM! Clean and well-organized imports

The imports are properly structured and all dependencies are relevant for the functionality.


32-33: Type safety improvement needed for namespace field

The namespace field using GraphQLJSONObject lacks type safety.


1-54: Excellent adherence to libs directory guidelines

The implementation follows best practices for:

  • Reusability: Clean model definition suitable for cross-app usage
  • TypeScript: Strong typing with GraphQL integration
  • Tree-shaking: Specific exports that enable effective bundling
apps/web/screens/Home/Home.tsx (1)

Line range hint 187-190: LGTM! Layout wrapper implementation is correct

The use of withMainLayout HOC with proper configuration follows NextJS best practices.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 comments (1)
apps/web/components/TableOfContents/TableOfContents.tsx (1)

Line range hint 13-15: Technical Debt: Remove ts-ignore comment.

The ts-ignore comment suggests there's a type safety issue that needs to be addressed. Consider fixing the underlying type issue instead of suppressing it.

Consider:

  1. Properly typing the json field in the Slice interface
  2. Using type guards to handle potential undefined values
  3. Adding proper null checks for the nested properties

Example approach:

interface SliceJson {
  title?: string;
  // add other potential json fields
}

// Then update the access to be type-safe:
text: (slice.json as SliceJson)?.title ?? slice.title ?? slice.leftTitle ?? ''
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c5db77 and 56eed5a.

📒 Files selected for processing (1)
  • apps/web/components/TableOfContents/TableOfContents.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/web/components/TableOfContents/TableOfContents.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."

@thorkellmani thorkellmani removed the deploy-feature Deploys features to dev label Dec 20, 2024
@thorkellmani thorkellmani marked this pull request as draft December 20, 2024 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants