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

chore(web): Stop importing GraphQL types from api/schema #17356

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

RunarVestmann
Copy link
Member

@RunarVestmann RunarVestmann commented Dec 27, 2024

Stop importing GraphQL types from api/schema

What

  • Import GraphQL types from @island.is/web/graphql/schema
  • Reorder imports

Why

  • GraphQL types are specifically generated for the web from queries, we should use those types instead of those from the api so we have a more correct type schema (since the queries might exclude fields)

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

  • New Features

    • Enhanced functionality in various components with updated imports and improved state management for better responsiveness and data handling.
    • Added new components and queries to the Home screen for improved layout and internationalization support.
    • Introduced pagination handling in the ApiCatalogue component for better data fetching.
  • Bug Fixes

    • Improved error handling for API queries to display appropriate messages based on loading states.
  • Refactor

    • Reorganized import statements across multiple components for clarity and maintainability.
    • Consolidated related imports in the PensionCalculator component for better organization.
    • Updated import sources for various types across multiple components to align with the new GraphQL schema.
  • Documentation

    • Updated method signatures and added new methods to reflect changes in component functionality and structure.

@RunarVestmann RunarVestmann requested review from a team as code owners December 27, 2024 10:26
@RunarVestmann RunarVestmann changed the title chore(web) Import grapql types from web chore(web): Import grapql types from web Dec 27, 2024
Copy link
Contributor

coderabbitai bot commented Dec 27, 2024

Walkthrough

This pull request involves a comprehensive update of import statements across multiple files in the web application. The primary change is migrating type and component imports from @island.is/api/schema to @island.is/web/graphql/schema. This refactoring affects various components and screens, including charts, forms, calculators, and list views. The changes primarily involve reorganizing import statements and updating import sources without significantly altering the underlying component logic or functionality.

Changes

File Path Change Summary
apps/web/components/Charts/v2/hooks/chart.ts Updated Chart type import source
apps/web/components/OfficialJournalOfIceland/OJOIAdvertCards.tsx Updated OfficialJournalOfIcelandAdvertsResponse import source
apps/web/components/ServiceWeb/Forms/StandardForm/StandardForm.tsx Updated multiple type imports, added new GraphQL query type imports
apps/web/components/TableOfContents/TableOfContents.tsx Updated Slice type import source
Multiple Fiskistofa components Updated various type imports from @island.is/api/schema to @island.is/web/graphql/schema
Multiple Syslumenn components Updated various type imports from @island.is/api/schema to @island.is/web/graphql/schema
apps/web/screens/Adgerdir/* Reorganized imports, updated type sources, added new components
apps/web/screens/Home/Home.tsx Updated imports, modified getProps method
apps/web/screens/LifeEvents/LifeEvents.tsx Moved ContentLanguage and QueryGetLifeEventsForOverviewArgs imports
apps/web/screens/Organization/* Updated various GraphQL schema type imports

Possibly related PRs

Suggested Reviewers

  • mannipje

Suggested labels

deploy-feature


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 generate docstrings to generate docstrings for this PR. (Beta)
  • @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.

@RunarVestmann RunarVestmann changed the title chore(web): Import grapql types from web chore(web): Stop importing GraphQL types from api/schema Dec 27, 2024
@RunarVestmann RunarVestmann added the automerge Merge this PR as soon as all checks pass label Dec 27, 2024
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

🧹 Nitpick comments (7)
apps/web/screens/Organization/StafraentIsland/ApiCatalogue.tsx (1)

49-49: Relative path import
Using relative paths can be fragile if the project structure changes. Consider an absolute import path if available.

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

33-33: Be mindful of document usage in a Next.js environment.

Directly accessing document may fail during SSR. Consider guarding with a runtime check (e.g., typeof window !== 'undefined') or a React effect hook if editing DOM elements. This prevents potential reference errors in a server-side context.


Line range hint 66-102: Assess naming of the getProps function for Next.js compatibility.

While defining Home.getProps can work as a custom utility, consider getStaticProps or getServerSideProps naming for conventional Next.js patterns. This can improve clarity and developer experience while ensuring correct SSR or SSG handling.

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

5-13: Ensure consistent styling and minimal complexity in imports.

The reorganized imports look clear. However, if these UI components are used widely across the codebase, consider dedicated UI modules for shared imports to keep each screen lightweight and consistent.

apps/web/components/ServiceWeb/Forms/StandardForm/StandardForm.tsx (2)

Line range hint 95-98: Conditional fields logic

The conditional rendering of fields based on categoryId and institutionSlugBelongsToMannaudstorg or institutionSligBelongsToDirectorateOfImmigration is well-defined. Double-check that these conditions are not duplicated in other parts of the code, to adhere to the DRY principle.


Line range hint 214-220: Enhanced validation checks

Use of Yup or a similar validation library can simplify complex validations and error messages, although the current approach with React Hook Form rules is acceptable. Consider centralizing repeated validation rules for maintainability.

apps/web/components/connected/fiskistofa/calculators/StraddlingStockCalculator/machine.ts (1)

1-3: Specify a more explicit type argument for ApolloClient
It's generally safer and clearer to replace ApolloClient<object> with a more specific type argument to improve maintainability and type safety.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 56493cc and 5351ede.

📒 Files selected for processing (25)
  • apps/web/components/Charts/v2/hooks/chart.ts (1 hunks)
  • apps/web/components/OfficialJournalOfIceland/OJOIAdvertCards.tsx (1 hunks)
  • apps/web/components/ServiceWeb/Forms/StandardForm/StandardForm.tsx (1 hunks)
  • apps/web/components/TableOfContents/TableOfContents.tsx (1 hunks)
  • apps/web/components/connected/electronicRegistrationStatistics/MonthlyStatistics/MonthlyStatistics.tsx (1 hunks)
  • apps/web/components/connected/fiskistofa/ShipSearch/ShipSearch.tsx (1 hunks)
  • apps/web/components/connected/fiskistofa/calculators/CatchQuotaCalculator/CatchQuotaCalculator.tsx (1 hunks)
  • apps/web/components/connected/fiskistofa/calculators/CatchQuotaCalculator/machine.ts (1 hunks)
  • apps/web/components/connected/fiskistofa/calculators/StraddlingStockCalculator/StraddlingStockCalculator.tsx (1 hunks)
  • apps/web/components/connected/fiskistofa/calculators/StraddlingStockCalculator/machine.ts (1 hunks)
  • apps/web/components/connected/syslumenn/TableLists/BrokersList/BrokersList.tsx (1 hunks)
  • apps/web/components/connected/syslumenn/TableLists/JourneymanList/JourneymanList.tsx (1 hunks)
  • apps/web/components/connected/syslumenn/TableLists/MasterList/MasterList.tsx (1 hunks)
  • apps/web/screens/Adgerdir/Article.tsx (1 hunks)
  • apps/web/screens/Adgerdir/Home.tsx (1 hunks)
  • apps/web/screens/Adgerdir/components/AdgerdirArticles/AdgerdirArticles.tsx (1 hunks)
  • apps/web/screens/Adgerdir/components/CardsSlider/CardsSlider.tsx (1 hunks)
  • apps/web/screens/Adgerdir/components/FeaturedNews/FeaturedNews.tsx (1 hunks)
  • apps/web/screens/Adgerdir/components/LatestNews/LatestNews.tsx (1 hunks)
  • apps/web/screens/Home/Home.tsx (1 hunks)
  • apps/web/screens/LifeEvents/LifeEvents.tsx (1 hunks)
  • apps/web/screens/Login/Login.tsx (1 hunks)
  • apps/web/screens/OfficialJournalOfIceland/hooks/useAdverts.ts (1 hunks)
  • apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx (1 hunks)
  • apps/web/screens/Organization/StafraentIsland/ApiCatalogue.tsx (2 hunks)
✅ Files skipped from review due to trivial changes (6)
  • apps/web/components/Charts/v2/hooks/chart.ts
  • apps/web/components/connected/fiskistofa/calculators/CatchQuotaCalculator/CatchQuotaCalculator.tsx
  • apps/web/screens/Adgerdir/components/LatestNews/LatestNews.tsx
  • apps/web/screens/Adgerdir/components/CardsSlider/CardsSlider.tsx
  • apps/web/screens/LifeEvents/LifeEvents.tsx
  • apps/web/screens/Organization/SocialInsuranceAdministration/PensionCalculator.tsx
🧰 Additional context used
📓 Path-based instructions (19)
apps/web/components/connected/syslumenn/TableLists/JourneymanList/JourneymanList.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/components/OfficialJournalOfIceland/OJOIAdvertCards.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/components/connected/fiskistofa/calculators/StraddlingStockCalculator/StraddlingStockCalculator.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/components/connected/syslumenn/TableLists/BrokersList/BrokersList.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/components/connected/syslumenn/TableLists/MasterList/MasterList.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/components/ServiceWeb/Forms/StandardForm/StandardForm.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/Adgerdir/components/FeaturedNews/FeaturedNews.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/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."
apps/web/components/connected/fiskistofa/ShipSearch/ShipSearch.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/OfficialJournalOfIceland/hooks/useAdverts.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/screens/Adgerdir/components/AdgerdirArticles/AdgerdirArticles.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/Adgerdir/Article.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/Adgerdir/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."
apps/web/screens/Login/Login.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/components/connected/fiskistofa/calculators/StraddlingStockCalculator/machine.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/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."
apps/web/components/connected/electronicRegistrationStatistics/MonthlyStatistics/MonthlyStatistics.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/Organization/StafraentIsland/ApiCatalogue.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/components/connected/fiskistofa/calculators/CatchQuotaCalculator/machine.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."
🔇 Additional comments (62)
apps/web/components/connected/fiskistofa/calculators/StraddlingStockCalculator/StraddlingStockCalculator.tsx (1)

16-16: Good alignment with the updated import path.

This change correctly updates the import path to reference @island.is/web/graphql/schema, aligning the component with newer GraphQL schema conventions. Confirm that all references to CatchQuotaCategory remain compatible with the new schema definitions throughout the codebase.

apps/web/screens/Adgerdir/components/AdgerdirArticles/AdgerdirArticles.tsx (16)

4-4: Use of useContext is correct and aligned with React best practices.
Everything looks good here for leveraging context from React.


7-7: Reintroduction of useState import is correct.
This import is essential for managing internal state.


10-10: Use of uniq is good for deduplication of tag IDs.
Lodash methods are well-suited for simplifying array manipulation. No issues found.


15-15: Inline import from Island UI is properly included.
Ensures consistent styling and layout.


17-17: Stack import from Island UI is properly included.
Allows vertical spacing between components.


18-18: Text import from Island UI is properly handled.
Matches the usage in the file.


19-19: Tiles import from Island UI is used correctly.
Enables a grid layout for cards.


23-23: ADGERDIR_COMPANIES_TAG_ID import follows consistency with constants.
This appears to be used correctly for filtering.


25-25: GraphQL schema import is aligned with the new path.
Importing AdgerdirPage and AdgerdirTag from @island.is/web/graphql/schema centralizes GraphQL type definitions in the web layer.


26-26: useNamespace import enhances localization and i18n usage.
Ensures that the namespace object is properly fetched and applied.


27-27: useLinkResolver import is consistent with linking patterns in the codebase.
Crucial for generating dynamic links as per the project’s routing strategy.


29-29: Button import from a custom UI directory is appropriate.
Maintains a consistent UI system across components.


30-30: Card import from a custom UI directory is consistent.
No issues found with structure or usage here.


31-31: ColorSchemeContext import ensures a consistent color scheme across the UI.
Leverages contextual theming effectively.


32-32: Tag import from a custom UI directory aligns with theming and styling.
Use of standardized design components is consistent with best practices.


34-34: CSS module import for AdgerdirArticles.css is correctly referenced.
No issues with naming or usage.

apps/web/components/connected/syslumenn/TableLists/BrokersList/BrokersList.tsx (1)

15-15: Verify new import path correctness.

The increased granularity of the new import path from @island.is/web/graphql/schema looks correct. Ensure that all references to the Query type from the old source have been updated across the codebase to avoid type mismatches.

apps/web/components/connected/electronicRegistrationStatistics/MonthlyStatistics/MonthlyStatistics.tsx (2)

14-14: Confirm alignment with updated endpoint.

Changing ConnectedComponent to the new schema path is consistent with project-wide restructuring. Verify downstream usage to confirm no references to the old import remain.
[approve]


18-18: Re-check local import usage.

This import is relocated, but the actual functionality appears unchanged. Confirm that CustomLegend is used as expected without introducing any regression.

apps/web/components/connected/syslumenn/TableLists/JourneymanList/JourneymanList.tsx (1)

21-21: Validate Journeyman types usage.

Introducing JourneymanLicence from the updated schema helps maintain consistency across the endpoints. Ensure that all Journeyman-related references have transitioned to the new import without leftover dependencies from the old schema.

apps/web/components/connected/syslumenn/TableLists/MasterList/MasterList.tsx (1)

21-21: Confirm full transition to the new schema.

Including MasterLicence from @island.is/web/graphql/schema replaces the old API import. Validate that references to MasterLicence are consistent throughout the logic and CSV exports.

apps/web/screens/Organization/StafraentIsland/ApiCatalogue.tsx (5)

3-7: Imports for Window Size and Router: Looks Good
These imports for useWindowSize, useRouter, useQuery, and @contentful/rich-text-types are well-structured and appropriate for their usage in the component.


8-14: GraphQL consts import
The newly introduced imports from @island.is/api-catalogue/consts appear consistent with the filter logic in this file.


25-31: Island UI theme and components
Well-organized imports for theme and specialized components. No issues here.


34-34: ContentLanguage & QueryGetApiCatalogueArgs updates
Switching to these updated schema imports ensures consistency across the codebase.

Also applies to: 37-37


42-48: Namespace & layout imports
These newly introduced hooks and layout imports align with project-wide best practices for code organization.

apps/web/components/connected/fiskistofa/calculators/CatchQuotaCalculator/machine.ts (3)

1-1: Apollo client and XState
The combined import of assign and createMachine from xstate alongside ApolloClient is fine, ensuring the state machine can interact with the GraphQL client.


4-4: sortAlpha utility
Using the shared sortAlpha utility is a straightforward approach to sorting logic. No concerns here.


12-18: Updated GraphQL schema imports
These schema type imports (e.g., QuotaType, Ship) are properly defined and used, enhancing your machine context's type safety.

apps/web/components/OfficialJournalOfIceland/OJOIAdvertCards.tsx (1)

3-3: Schema import path
Switching OfficialJournalOfIcelandAdvertsResponse to @island.is/web/graphql/schema is consistent with the general schema consolidation in this PR.

apps/web/components/TableOfContents/TableOfContents.tsx (1)

4-4: Slice type path
Importing Slice from @island.is/web/graphql/schema fits well with the rest of the codebase refactoring.

apps/web/screens/OfficialJournalOfIceland/hooks/useAdverts.ts (1)

6-6: Adverts schema import path
Replacing the advert response types to reference @island.is/web/graphql/schema promotes consistency across the application.

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

2-13: Check alignment with Next.js best practices for import structure and maintainability.

These extensive import changes appear consistent, but ensure all exposed modules from @island.is/web/components and @island.is/web/graphql/schema remain consistent across the codebase. Confirm that none of the old references from @island.is/api/schema are still in use in this file or other related modules.


19-22: Confirm the correctness of new GraphQL type imports.

You switched to @island.is/web/graphql/schema. Verify any differences in field names or structures. This is especially important for typed queries or fragment usage, ensuring alignment with the updated schema.

apps/web/components/connected/fiskistofa/ShipSearch/ShipSearch.tsx (1)

15-19: Verify GraphQL schema coherence after import path change.

Since the import path changed from @island.is/api/schema to @island.is/web/graphql/schema, confirm that all other references to the old schema have been removed or updated. Also verify that the new schema fields (e.g., FiskistofaShipBasicInfoResponse) match your query shape to avoid runtime type mismatches.

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

15-15: Check SSR usage of client-specific globals.

webLoginButtonSelect and window?.plausible rely on client-side resources. If this page renders SSR, ensure you have appropriate guards or fallback logic, as these references may be undefined at build time.

apps/web/screens/Adgerdir/components/FeaturedNews/FeaturedNews.tsx (1)

7-8: Validate accurate usage of newly imported UI components and GraphQL types.

The addition of Button, GridColumn, and Stack alongside switching from @island.is/api/schema to @island.is/web/graphql/schema is fine, but confirm that the News type fields match the new schema and no required fields were dropped in the transition.

Also applies to: 12-16

apps/web/screens/Adgerdir/Article.tsx (10)

2-2: Ensure consistent usage of React functionality

Imports from React, including hooks like useEffect, useRef, and useState, look fine. Verify that these hooks are used consistently and that they address any potential memory leak or unmounted component issues, especially if the lifecycle of the rendered DOM portal is dynamic.


6-13: Adhere to Next.js file structure conventions

These imports from @island.is/island-ui and @island.is/web/graphql are appropriate for a Next.js app within apps/web. They align well with the recommended approach of local component usage and typed GraphQL schema usage.


14-16: Social sharing strategy

The HeadWithSocialSharing import is a good approach for setting up social media metadata. Just ensure that all essential meta tags (e.g., og:image, og:title) are populated consistently across your site.


17-22: Use typed query arguments for clarity

Switching to @island.is/web/graphql/schema for QueryGetAdgerdirPageArgs, QueryGetAdgerdirPagesArgs, QueryGetAdgerdirTagsArgs, and QueryGetNamespaceArgs is beneficial. Keeping all your type definitions in one central location can reduce confusion. Continue verifying that all references to these query argument types have been updated to prevent type mismatches.


23-25: Namespace hook usage

useNamespace is effectively used to manage localized content. Confirm that each string key has a matching fallback in case the API fails to deliver certain fields.


26-29: Layout consistency

SidebarLayout and CustomNextError usage is correct. Ensure each layout component remains consistent with your Next.js routing and static generation patterns (e.g., ensuring getProps or getStaticProps usage where relevant).


30-35: Link resolution logic

Using useLinkResolver to dynamically generate URL paths is a good pattern. Confirm that slug-based links in the code are tested to avoid 404s from unexpected or malformed slugs.


37-41: Component segmentation

Components like AdgerdirArticles, Breadcrumbs, ColorSchemeContext, and ProcessEntry are organized well. Maintaining a clear file structure in apps/web/screens/Adgerdir/ is helpful for discoverability.


Line range hint 46-46: Exporting typed Screen components

Leveraging Screen<AdgerdirArticleProps> ensures you retain strong type safety for your Next.js pages. This is in line with TypeScript best practices for Next.js apps.


Line range hint 78-136: Error handling in getProps

Throwing a CustomNextError if getAdgerdirPage is missing elegantly handles 404 scenarios. This is aligned with standard Next.js error handling. Ensure the backend or headless CMS always returns expected data or a well-defined null object if not found, to avoid unexpected 500s.

apps/web/screens/Adgerdir/Home.tsx (8)

5-15: Consistent usage of island.is UI components

Using Slice from @island.is/island-ui/contentful and layout primitives (e.g., Box, Stack) helps maintain a uniform design system. Ensure there are no custom, untested modifications to these components that could affect performance or behavior.


17-22: Accurate locale usage

Leveraging Locale from @island.is/shared/types and useI18n helps ensure text is displayed correctly. Confirm the fallback flow if activeLocale is undefined or if content is not available.


26-31: GraphQL schema changes

References to ContentLanguage, QueryGetAdgerdirPagesArgs, QueryGetAdgerdirTagsArgs, QueryGetGroupedMenuArgs, and QueryGetNamespaceArgs from @island.is/web/graphql/schema keep your data layer coherent. Consistency across the entire web app is crucial, so continue verifying all references are kept in sync.


37-39: Hook usage check

useNamespace, useLinkResolver, and useI18n are used properly. Validate that each hook’s return value is consumed in a type-safe manner, especially if a particular field might be missing or null.


46-47: Importing local utilities

Local utility imports (formatMegaMenuCategoryLinks, formatMegaMenuLinks) keep your code DRY and organized. Ensure these utilities remain well tested so that changes to the link formatting logic don’t introduce regressions.


48-55: GraphQL queries structure

The queries for GET_ADGERDIR_FRONTPAGE_QUERY, GET_ADGERDIR_PAGES_QUERY, GET_ADGERDIR_TAGS_QUERY, GET_CATEGORIES_QUERY, and GET_GROUPED_MENU_QUERY show that you’re modularizing your GraphQL operations properly. Continue verifying that the relevant resolvers exist and respond with the expected schema on the server side.


58-60: Breadcrumbs usage

The Breadcrumbs component is used effectively to show user navigation context. Confirm that the routing definitions or link resolvers handle edge cases, like nested routes or missing data.


Line range hint 136-177: getProps + Mega Menu data

This approach effectively fetches all required data for the Home component, including handling multiple parallel GraphQL queries and merging their results. Keep in mind potential performance implications regarding how large these queries can get.

apps/web/components/ServiceWeb/Forms/StandardForm/StandardForm.tsx (3)

Line range hint 33-41: Migration to @island.is/web/graphql/schema

Switching your types (Organizations, SupportCategory, etc.) and introducing GetSupportSearchResultsQuery/Variables from the new schema is consistent with the rest of the PR. Verify that all references to the old schema have been removed to avoid confusion or mismatched imports.


Line range hint 108-112: Debouncing side effects

Using useDebounce for search subject changes is a good approach for performance. Continue ensuring that once the subject is cleared, the suggestions state is also reset to avoid leftover references or memory overhead.


Line range hint 267-281: Form submission logic

Appending user-entered fields into the message and sending that to onSubmit is straightforward. Be mindful of data privacy concerns, ensuring that no sensitive user data is improperly stored or logged, and that the email body is sanitized if needed.

apps/web/components/connected/fiskistofa/calculators/StraddlingStockCalculator/machine.ts (3)

4-5: Imports for Next.js
These imports look good. They adhere to the recommended approach of modularizing utilities and custom Apollo client initialization for Next.js.


9-9: Confirm usage of FiskistofaQuotaTypeResponse
Ensure references to this type are consistently updated and no stale imports from @island.is/api/schema remain.

✅ Verification successful

No stale imports found - type is correctly imported

The verification shows that FiskistofaQuotaTypeResponse is properly imported from local module paths:

  • In resolver: ./models/quotaType
  • Defined in: libs/api/domains/fiskistofa/src/lib/models/quotaType.ts

There are no instances of importing this type from @island.is/api/schema, and all usages are consistent with the local module imports.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Searching for old imports of FiskistofaQuotaTypeResponse from @island.is/api/schema
rg "FiskistofaQuotaTypeResponse" -A 3 | rg "@island.is/api/schema"

Length of output: 64


Script:

#!/bin/bash
# Let's first find all occurrences of FiskistofaQuotaTypeResponse to understand its usage
rg "FiskistofaQuotaTypeResponse" -B 2 -A 2

Length of output: 4613


12-14: Migrate all GraphQL imports to the new schema path
The import path for GraphQL types has changed to @island.is/web/graphql/schema. Verify there are no outdated references to the old schema.

@datadog-island-is
Copy link

datadog-island-is bot commented Dec 27, 2024

Datadog Report

All test runs f0cd1d9 🔗

10 Total Test Services: 0 Failed, 10 Passed
🔻 Test Sessions change in coverage: 1 decreased (-0.02%), 3 increased, 196 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-backend 0 0 0 63 0 22.25s N/A Link
air-discount-scheme-web 0 0 0 2 0 6.62s N/A Link
api 0 0 0 4 0 2.42s N/A Link
api-catalogue-services 0 0 0 23 0 9.03s N/A Link
api-domains-air-discount-scheme 0 0 0 6 0 16.51s N/A Link
api-domains-assets 0 0 0 3 0 9.61s N/A Link
api-domains-auth-admin 0 0 0 18 0 10.19s N/A Link
api-domains-communications 0 0 0 5 0 30.22s 1 no change Link
api-domains-criminal-record 0 0 0 5 0 9.59s N/A Link
api-domains-driving-license 0 0 0 23 0 27.39s N/A Link

🔻 Code Coverage Decreases vs Default Branch (1)

  • services-university-gateway - jest 45.26% (-0.02%) - Details

Copy link

codecov bot commented Dec 27, 2024

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 35.68%. Comparing base (2699012) to head (f0972ee).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...web/components/connected/syslumenn/utils/search.ts 0.00% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17356      +/-   ##
==========================================
- Coverage   35.68%   35.68%   -0.01%     
==========================================
  Files        6931     6931              
  Lines      148782   148785       +3     
  Branches    42509    42509              
==========================================
  Hits        53091    53091              
- Misses      95691    95694       +3     
Flag Coverage Δ
air-discount-scheme-backend 48.24% <ø> (ø)
air-discount-scheme-web 0.00% <ø> (ø)
api 3.33% <ø> (ø)
api-catalogue-services 75.81% <ø> (ø)
api-domains-air-discount-scheme 37.99% <ø> (ø)
api-domains-assets 26.71% <ø> (ø)
api-domains-auth-admin 48.49% <ø> (ø)
api-domains-communications 39.42% <ø> (ø)
api-domains-criminal-record 48.03% <ø> (ø)
api-domains-driving-license 44.86% <ø> (ø)
api-domains-education 31.27% <ø> (ø)
api-domains-health-insurance 35.47% <ø> (ø)
api-domains-mortgage-certificate 35.24% <ø> (+0.10%) ⬆️
api-domains-payment-schedule 42.12% <ø> (ø)
application-api-files 61.90% <ø> (ø)
application-core 75.72% <ø> (ø)
application-system-api 38.73% <ø> (-0.01%) ⬇️
application-template-api-modules 27.67% <ø> (-0.02%) ⬇️
application-templates-accident-notification 27.61% <ø> (ø)
application-templates-car-recycling 3.12% <ø> (ø)
application-templates-criminal-record 25.83% <ø> (ø)
application-templates-driving-license 18.15% <ø> (ø)
application-templates-estate 13.66% <ø> (ø)
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.68% <ø> (ø)
application-templates-parental-leave 29.95% <ø> (ø)
application-types 6.51% <ø> (ø)
application-ui-components 1.22% <ø> (ø)
application-ui-shell 22.26% <ø> (ø)
auth-admin-web 2.43% <ø> (ø)
auth-nest-tools 31.52% <ø> (ø)
clients-charge-fjs-v2 28.88% <ø> (ø)
clients-driving-license 40.91% <ø> (ø)
clients-driving-license-book 43.92% <ø> (ø)
clients-financial-statements-inao 49.70% <ø> (ø)
clients-license-client 1.26% <ø> (ø)
clients-middlewares 73.50% <ø> (+0.57%) ⬆️
clients-regulations 43.00% <ø> (ø)
clients-rsk-company-registry 31.18% <ø> (ø)
clients-rsk-personal-tax-return 38.32% <ø> (ø)
clients-smartsolutions 12.77% <ø> (ø)
clients-syslumenn 49.47% <ø> (ø)
clients-zendesk 50.58% <ø> (ø)
cms 0.39% <ø> (ø)
cms-translations 38.75% <ø> (ø)
content-search-toolkit 8.16% <ø> (ø)
contentful-apps 4.71% <ø> (ø)
dokobit-signing 62.18% <ø> (ø)
email-service 60.15% <ø> (ø)
file-storage 45.97% <ø> (ø)
financial-aid-backend 51.42% <ø> (ø)
financial-aid-shared 17.88% <ø> (ø)
icelandic-names-registry-backend 54.33% <ø> (ø)
infra-nest-server 48.06% <ø> (ø)
infra-tracing 69.94% <ø> (ø)
island-ui-core 30.42% <ø> (ø)
judicial-system-api 20.20% <ø> (ø)
judicial-system-audit-trail 69.02% <ø> (ø)
judicial-system-backend 55.98% <ø> (ø)
judicial-system-message 66.60% <ø> (ø)
judicial-system-message-handler 48.33% <ø> (ø)
judicial-system-scheduler 71.15% <ø> (ø)
judicial-system-web 27.72% <ø> (ø)
license-api 42.94% <ø> (+0.07%) ⬆️
localization 10.15% <ø> (ø)
logging 58.02% <ø> (ø)
message-queue 67.05% <ø> (ø)
nest-audit 65.78% <ø> (ø)
nest-aws 51.93% <ø> (ø)
nest-config 76.45% <ø> (ø)
nest-core 53.16% <ø> (ø)
nest-feature-flags 51.06% <ø> (ø)
nest-sequelize 94.44% <ø> (ø)
nest-swagger 51.13% <ø> (ø)
nova-sms 61.52% <ø> (ø)
portals-admin-regulations-admin 1.80% <ø> (ø)
portals-core 19.58% <ø> (ø)
services-auth-admin-api 52.54% <ø> (+0.01%) ⬆️
services-auth-delegation-api 58.55% <ø> (ø)
services-auth-ids-api 52.56% <ø> (+<0.01%) ⬆️
services-auth-public-api 49.42% <ø> (ø)
services-sessions 65.36% <ø> (+0.04%) ⬆️
services-university-gateway 49.43% <ø> (-0.09%) ⬇️
services-user-notification 46.53% <ø> (-0.01%) ⬇️
services-user-profile 56.97% <ø> (+0.08%) ⬆️
shared-components 29.49% <ø> (ø)
shared-form-fields 33.38% <ø> (ø)
shared-utils 27.69% <ø> (ø)
skilavottord-ws 24.26% <ø> (ø)
web 2.40% <0.00%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
apps/web/components/Charts/v2/hooks/chart.ts 0.00% <ø> (ø)
...nents/OfficialJournalOfIceland/OJOIAdvertCards.tsx 0.00% <ø> (ø)
...nts/ServiceWeb/Forms/StandardForm/StandardForm.tsx 0.00% <ø> (ø)
...web/components/TableOfContents/TableOfContents.tsx 0.00% <ø> (ø)
...Statistics/MonthlyStatistics/MonthlyStatistics.tsx 0.00% <ø> (ø)
...nts/connected/fiskistofa/ShipSearch/ShipSearch.tsx 0.00% <ø> (ø)
...tors/CatchQuotaCalculator/CatchQuotaCalculator.tsx 0.00% <ø> (ø)
...istofa/calculators/CatchQuotaCalculator/machine.ts 0.00% <ø> (ø)
...dlingStockCalculator/StraddlingStockCalculator.tsx 0.00% <ø> (ø)
...a/calculators/StraddlingStockCalculator/machine.ts 0.00% <ø> (ø)
... and 16 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 56493cc...f0972ee. Read the comment docs.

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: 2

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 5351ede and 9522bae.

📒 Files selected for processing (2)
  • apps/web/components/connected/syslumenn/TableLists/JourneymanList/JourneymanList.tsx (1 hunks)
  • apps/web/components/connected/syslumenn/utils/search.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/components/connected/syslumenn/TableLists/JourneymanList/JourneymanList.tsx
🧰 Additional context used
📓 Path-based instructions (1)
apps/web/components/connected/syslumenn/utils/search.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."
🔇 Additional comments (1)
apps/web/components/connected/syslumenn/utils/search.ts (1)

22-24: Remove Generic Constraint with Caution

By removing the Record<string, string | null> constraint from <T>, you gain flexibility but also lose built-in guarantees that each key property is a string. This can lead to runtime errors if items in the list contain non-string or missing values. Consider whether you need an additional type guard or runtime check to ensure that item[key] is safely convertible to a string.

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/Adgerdir/Home.tsx (3)

Line range hint 61-66: Remove ts-ignore and properly type megaMenuData

The megaMenuData prop uses a ts-ignore comment. Consider properly typing this prop to maintain type safety.

interface HomeProps {
  frontpage: Query['getAdgerdirFrontpage']
  pages: Query['getAdgerdirPages']
  tags: Query['getAdgerdirTags']
  namespace: Query['getNamespace']
  megaMenuData: {
    menus: Array<{
      title: string
      menuLinks: Array<{
        // Add appropriate types based on actual data structure
      }>
    }>
  }
}

Line range hint 89-91: Remove unnecessary ts-ignore comments

There are multiple ts-ignore comments that should be addressed. Consider properly typing the namespace and items to maintain type safety.

const n = useNamespace(namespace as Record<string, string>)
const { items: tagsItems } = tags as { items: Array</* appropriate type */> }

Also applies to: 96-98


Line range hint 234-308: Add error handling for GraphQL queries

The getProps method executes multiple GraphQL queries in parallel but lacks error handling. Consider adding try-catch blocks and proper error handling.

Home.getProps = async ({ apolloClient, locale }) => {
  try {
    const [
      frontpageResult,
      tagsResult,
      pagesResult,
      namespace,
      megaMenuData,
      categories,
    ] = await Promise.all([
      // ... existing queries ...
    ])

    // ... existing transformation ...

    return {
      // ... existing return ...
    }
  } catch (error) {
    console.error('Failed to fetch data:', error)
    // Handle error appropriately
    throw error
  }
}
🧹 Nitpick comments (3)
apps/web/screens/Adgerdir/Home.tsx (3)

17-33: Consider grouping type imports for better maintainability

The type imports from @island.is/web/graphql/schema could be organized better. Consider grouping all Query-related types together.

import { Locale } from '@island.is/shared/types'
import {
- ContentLanguage,
- GetArticleCategoriesQuery,
- GetGroupedMenuQuery,
  Query,
  QueryGetAdgerdirPagesArgs,
  QueryGetAdgerdirTagsArgs,
  QueryGetArticleCategoriesArgs,
  QueryGetGroupedMenuArgs,
  QueryGetNamespaceArgs,
+ ContentLanguage,
+ GetArticleCategoriesQuery,
+ GetGroupedMenuQuery,
} from '@island.is/web/graphql/schema'

Line range hint 182-227: Consider extracting slice rendering logic

The slice rendering logic in the map function is complex and could be extracted into a separate component for better maintainability.

interface SliceProps {
  slice: AdgerdirFeaturedNewsSlice | AdgerdirGroupSlice
  index: number
  groupSliceCount: number
}

const SliceRenderer: React.FC<SliceProps> = ({ slice, index, groupSliceCount }) => {
  const colorScheme = groupSliceCount % 2 ? 'blue' : 'green'
  
  switch (slice.__typename) {
    case 'AdgerdirFeaturedNewsSlice':
      return <FeaturedNews key={index} items={slice.featured} />
    case 'AdgerdirGroupSlice':
      return (
        <CovidColorSchemeContext.Provider value={{ colorScheme }}>
          {/* ... existing GroupSlice rendering ... */}
        </CovidColorSchemeContext.Provider>
      )
    default:
      return null
  }
}

Line range hint 310-326: Extract mega menu data transformation

The mega menu data transformation logic could be extracted into a utility function for better reusability and testing.

// utils/megaMenu.ts
export const transformMegaMenuData = (
  locale: Locale,
  megaMenuData: any,
  categories: any,
) => ({
  asideTopLinks: formatMegaMenuLinks(
    locale,
    megaMenuData?.menus?.[0]?.menuLinks ?? [],
  ),
  asideBottomTitle: megaMenuData?.menus?.[1]?.title ?? '',
  asideBottomLinks: formatMegaMenuLinks(
    locale,
    megaMenuData?.menus?.[1]?.menuLinks ?? [],
  ),
  mainLinks: formatMegaMenuCategoryLinks(locale, categories),
})
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 9522bae and f0972ee.

📒 Files selected for processing (2)
  • apps/web/components/ServiceWeb/Forms/StandardForm/StandardForm.tsx (1 hunks)
  • apps/web/screens/Adgerdir/Home.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/components/ServiceWeb/Forms/StandardForm/StandardForm.tsx
🧰 Additional context used
📓 Path-based instructions (1)
apps/web/screens/Adgerdir/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."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this PR as soon as all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant