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(devex): Local mocking of calls to the national registry #15523

Merged
merged 32 commits into from
Aug 27, 2024

Conversation

norda-gunni
Copy link
Contributor

@norda-gunni norda-gunni commented Jul 16, 2024

...

Attach a link to issue if relevant

What

Added a local mock proxy to enable local mocking of requests to the national registry.

Why

The national registry api tends to be kind of slow to respond to requests so enabling local mocking makes for a much snappier local dev experience. A side benefit is less load on the national registry dev env.

The same setup can also be used to mock any other service.

Screenshots / Gifs

Attach Screenshots / Gifs to help reviewers understand the scope of the pull request

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

    • Introduced a mock service for the National Registry, enabling simulated responses for retrieving individual data such as personal details, marital status, and health insurance status.
    • Added comprehensive documentation for setting up and using the National Registry mock endpoints, including prerequisites, setup instructions, and benefits of local mocking.
  • Dependencies

    • Upgraded framer-motion from 6.5.1 to 9.0.0.
    • Added @mockoon/cli dependency (version 8.3.0).

@norda-gunni norda-gunni requested a review from a team as a code owner July 16, 2024 12:22
Copy link
Contributor

coderabbitai bot commented Jul 16, 2024

Walkthrough

The recent changes introduce a mock service for the National Registry endpoints, facilitating local development and testing. The updates include a new mock configuration file and a README guide on using these mocks. Additionally, the package.json file has been updated to reflect changes in dependencies, including the addition of @mockoon/cli and an upgrade of framer-motion.

Changes

File(s) Change Summary
mocks/README.md Added a guide on mocking national registry XROAD endpoints, covering prerequisites, starting the mock server, available mocks, and benefits of local mocking.
mocks/national-registry.json Introduced a mock service for the National Registry, including routes for individual data retrieval with detailed response data, headers, and status codes.
package.json Updated dependencies: framer-motion upgraded from 6.5.1 to 9.0.0, and added a new dependency @mockoon/cli version 8.3.0.

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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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

Outside diff range, codebase verification and nitpick comments (3)
mocks/README.md (3)

5-5: Consider adding a comma for clarity.

The sentence might benefit from a comma after "port 8082" for better readability.

- Since the mock will be listening on port `8081` the port forwarding for xroad needs to be listening on port `8082` as that is where the mock server will forward requests it does not have mock responses for
  Since the mock will be listening on port `8081`, the port forwarding for xroad needs to be listening on port `8082` as that is where the mock server will forward requests it does not have mock responses for
Tools
LanguageTool

[uncategorized] ~5-~5: Possible missing comma found.
Context: ...t is where the mock server will forward requests it does not have mock responses for ##...

(AI_HYDRA_LEO_MISSING_COMMA)


13-13: Add a comma for grammatical correctness.

There should be a comma after the introductory phrase "Currently".

- Currently only mock responses for the national registry are included.
  Currently, only mock responses for the national registry are included.

25-25: Consider adding a comma for clarity.

Adding a comma after "response time" could improve the readability of the sentence.

- No, one of the benefits of mocking locally is way shorter response time and to get that we need to mock even if the underlying service is up.
  No, one of the benefits of mocking locally is way shorter response time, and to get that we need to mock even if the underlying service is up.
Tools
LanguageTool

[uncategorized] ~25-~25: Possible missing comma found.
Context: ...mocking locally is way shorter response time and to get that we need to mock even if...

(AI_HYDRA_LEO_MISSING_COMMA)


[style] ~25-~25: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...horter response time and to get that we need to mock even if the underlying service is ...

(REP_NEED_TO_VB)

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 143c0e0 and f91fd0d.

Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing (3)
  • mocks/README.md (1 hunks)
  • mocks/national-registry.json (1 hunks)
  • package.json (2 hunks)
Additional context used
LanguageTool
mocks/README.md

[uncategorized] ~5-~5: Possible missing comma found.
Context: ...t is where the mock server will forward requests it does not have mock responses for ##...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~12-~12: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...k-national-registry` ## Current mocks Currently only mock responses for the national re...

(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)


[style] ~21-~21: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...Gervimaður Færeyjar fake person. If you need to mock other fake prersons you can downlo...

(REP_NEED_TO_VB)


[uncategorized] ~21-~21: Possible missing comma found.
Context: ... person. If you need to mock other fake prersons you can download the [mockoon app](http...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~25-~25: Possible missing comma found.
Context: ...mocking locally is way shorter response time and to get that we need to mock even if...

(AI_HYDRA_LEO_MISSING_COMMA)


[style] ~25-~25: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...horter response time and to get that we need to mock even if the underlying service is ...

(REP_NEED_TO_VB)

Additional comments not posted (3)
package.json (2)

340-340: Addition of @mockoon/cli.

The addition of @mockoon/cli supports the new mocking functionality. Ensure that this tool integrates well with the existing development workflow and that its version is compatible with other project dependencies.


184-184: Major version update for framer-motion.

The update from 6.5.1 to 9.0.0 is a major version change. It's important to ensure that this doesn't introduce any breaking changes or compatibility issues with existing animations.

mocks/national-registry.json (1)

1-705: Review of the mock configuration file.

The configuration appears comprehensive and well-structured. However, ensure that the use of wildcards in CORS settings (Access-Control-Allow-Origin: *) is intentional and acceptable for your security policy.

mocks/README.md Outdated Show resolved Hide resolved
mocks/README.md Show resolved Hide resolved
mocks/README.md Outdated Show resolved Hide resolved
Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
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: 6

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f91fd0d and 1c1c8bc.

Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing (2)
  • mocks/README.md (1 hunks)
  • package.json (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • package.json
Additional context used
LanguageTool
mocks/README.md

[uncategorized] ~5-~5: Possible missing comma found.
Context: ...t is where the mock server will forward requests it does not have mock responses for ##...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~12-~12: A comma may be missing after the conjunctive/linking adverb ‘Currently’.
Context: ...k-national-registry` ## Current mocks Currently only mock responses for the national re...

(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)


[style] ~21-~21: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...Gervimaður Færeyjar fake person. If you need to mock other fake persons, you can downlo...

(REP_NEED_TO_VB)


[uncategorized] ~25-~25: Possible missing comma found.
Context: ...mocking locally is way shorter response time and to get that we need to mock even if...

(AI_HYDRA_LEO_MISSING_COMMA)


[style] ~25-~25: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...horter response time and to get that we need to mock even if the underlying service is ...

(REP_NEED_TO_VB)

mocks/README.md Show resolved Hide resolved
mocks/README.md Outdated Show resolved Hide resolved
mocks/README.md Outdated Show resolved Hide resolved
mocks/README.md Outdated Show resolved Hide resolved
mocks/README.md Outdated Show resolved Hide resolved
norda-gunni and others added 5 commits July 16, 2024 13:00
Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Copy link
Member

@baering baering left a comment

Choose a reason for hiding this comment

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

A good start. I added some comments for you to review.

I'm wondering if there could be a simple path to ensuring that the mocks have the same schema as the clients to make future additions and maintenance simpler.

Thinking 'out loud' can we auto generate the mock files from the client json and perhaps use examples as mock data? If generating them automatically is unrealistic, can we do it programmatically so we can import the generated model types, in this case from libs/clients/national-registry/v2/gen/fetch/models?

mocks/README.md Show resolved Hide resolved
mocks/README.md Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
yarn.lock Outdated Show resolved Hide resolved
@baering
Copy link
Member

baering commented Jul 16, 2024

It looks like we already are using a tool for mocking APIs, Mountebank.

Currently used to mock xroad in tests

@norda-gunni
Copy link
Contributor Author

norda-gunni commented Jul 17, 2024

As of May 19th of this year mountebank is no longer being maintained. See the project's readme. Mockoon is also very user friendly, offering gui apps, a cli and a docker container.

@norda-gunni norda-gunni requested a review from baering July 17, 2024 11:19
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: 7

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1c1c8bc and 2225e93.

Files selected for processing (1)
  • mocks/README.md (1 hunks)
Additional context used
LanguageTool
mocks/README.md

[uncategorized] ~13-~13: Possible missing comma found.
Context: ...ffic to port 8082. For more in depth instructions you can check out the [mockoon site](ht...

(AI_HYDRA_LEO_MISSING_COMMA)

mocks/README.md Show resolved Hide resolved
mocks/README.md Outdated Show resolved Hide resolved
mocks/README.md Show resolved Hide resolved
mocks/README.md Show resolved Hide resolved
mocks/README.md Show resolved Hide resolved
mocks/README.md Outdated Show resolved Hide resolved
mocks/README.md Outdated Show resolved Hide resolved
andes-it and others added 2 commits July 17, 2024 11:22
Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
norda-gunni and others added 2 commits July 17, 2024 11:26
Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
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, codebase verification and nitpick comments (1)
mocks/README.md (1)

7-7: Add missing commas for clarity.

There seems to be a missing comma after "Alternatively" which can improve the readability of the instructions.

- Alternatively if you use kubectl and socat just replace `8081:80` with `8082:80`.
  Alternatively, if you use kubectl and socat, just replace `8081:80` with `8082:80`.
Tools
LanguageTool

[uncategorized] ~7-~7: A comma might be missing here.
Context: ... like so yarn proxies xroad --p 8082. Alternatively if you use kubectl and socat just repla...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)


[uncategorized] ~7-~7: A comma might be missing here.
Context: .... Alternatively if you use kubectl and socat just replace 8081:80with8082:80`. ...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between ea80c7f and 9d8806c.

Files selected for processing (1)
  • mocks/README.md (1 hunks)
Additional context used
LanguageTool
mocks/README.md

[uncategorized] ~7-~7: A comma might be missing here.
Context: ... like so yarn proxies xroad --p 8082. Alternatively if you use kubectl and socat just repla...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)


[uncategorized] ~7-~7: A comma might be missing here.
Context: .... Alternatively if you use kubectl and socat just replace 8081:80with8082:80`. ...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)

Additional comments not posted (6)
mocks/README.md (6)

5-7: Clarify port forwarding setup.

The instructions on setting up port forwarding are clear, but it might be beneficial to include a note about why port 8082 is specifically needed, especially since 8081 is used for Mockoon. This would help developers understand the network setup better.

Would you like me to add this clarification?

Tools
LanguageTool

[uncategorized] ~7-~7: A comma might be missing here.
Context: ... like so yarn proxies xroad --p 8082. Alternatively if you use kubectl and socat just repla...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)


[uncategorized] ~7-~7: A comma might be missing here.
Context: .... Alternatively if you use kubectl and socat just replace 8081:80with8082:80`. ...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)


11-11: Clarify the mock server startup command.

It might be helpful to specify where this command should be run or if there are any specific directory or setup prerequisites.

Would you like me to add more detailed instructions on where and how to run this command?


17-17: Specify version for clarity.

It's mentioned that only mock responses for the national registry V2 are included. It might be useful to specify what aspects of V2 are covered to avoid any confusion for developers who are unfamiliar with the version details.

Would you like help detailing the V2 coverage in the mocks?


21-21: Expand on transparent proxying.

The description of how Mockoon handles unmocked requests is succinct. However, adding a brief explanation of how transparent proxying works could provide valuable context for developers new to the concept.

Would you like me to include a short explanation or diagram here?


25-25: Clarify the setup for additional mocks.

The instructions for setting up additional mocks are clear, but it could be beneficial to include examples of common scenarios or links to tutorials for less experienced users.

Would you like me to add examples or further resources?


29-29: Highlight the benefits of local mocking.

The last paragraph does a good job of explaining the benefits of local mocking. Consider adding a brief note on how this can also help in scenarios where the national registry is unavailable due to maintenance or outages, enhancing the understanding of its utility.

Would you like me to draft this addition?

@datadog-island-is
Copy link

datadog-island-is bot commented Jul 29, 2024

Datadog Report

All test runs e88eb81 🔗

101 Total Test Services: 0 Failed, 99 Passed
🔻 Test Sessions change in coverage: 2 decreased, 6 increased, 192 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 81 0 30.14s N/A Link
air-discount-scheme-web 0 0 0 2 0 8.63s N/A Link
api 0 0 0 4 0 2.89s N/A Link
api-catalogue-services 0 0 0 23 0 12.31s N/A Link
api-domains-air-discount-scheme 0 0 0 6 0 23.65s N/A Link
api-domains-assets 0 0 0 3 0 15.02s N/A Link
api-domains-auth-admin 0 0 0 18 0 12.76s 1 no change Link
api-domains-communications 0 0 0 5 0 38.73s N/A Link
api-domains-criminal-record 0 0 0 5 0 9.55s N/A Link
api-domains-driving-license 0 0 0 23 0 35.77s N/A Link

🔻 Code Coverage Decreases vs Default Branch (2)

  • services-auth-public-api - jest 51.84% (-0.8%) - Details
  • application-template-api-modules - jest 25.68% (-0.01%) - Details

Copy link

codecov bot commented Jul 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.97%. Comparing base (2277755) to head (7385730).
Report is 4 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #15523    /-   ##
=======================================
  Coverage   36.97%   36.97%           
=======================================
  Files        6625     6625           
  Lines      135767   135767           
  Branches    38631    38631           
=======================================
  Hits        50196    50197     1     
  Misses      85571    85570    -1     
Flag Coverage Δ
air-discount-scheme-web 0.00% <ø> (ø)
api 3.42% <ø> (ø)
api-catalogue-services 77.85% <ø> (ø)
api-domains-air-discount-scheme 37.07% <ø> (ø)
api-domains-assets 26.71% <ø> (ø)
api-domains-communications 40.60% <ø> (ø)
api-domains-criminal-record 48.16% <ø> (ø)
api-domains-driving-license 44.53% <ø> (ø)
api-domains-education 33.86% <ø> (ø)
api-domains-health-insurance 35.07% <ø> (ø)
api-domains-mortgage-certificate 36.04% <ø> (ø)
api-domains-payment-schedule 41.34% <ø> (ø)
application-api-files 57.92% <ø> (ø)
application-core 72.24% <ø> ( 0.33%) ⬆️
application-system-api 41.86% <ø> (-0.01%) ⬇️
application-template-api-modules 23.69% <ø> ( 0.01%) ⬆️
application-templates-accident-notification 19.85% <ø> (ø)
application-templates-car-recycling 3.12% <ø> (ø)
application-templates-criminal-record 22.15% <ø> (ø)
application-templates-driving-license 16.00% <ø> (ø)
application-templates-estate 11.88% <ø> (ø)
application-templates-example-payment 20.82% <ø> (ø)
application-templates-general-petition 18.89% <ø> (ø)
application-templates-inheritance-report 4.63% <ø> (ø)
application-templates-marriage-conditions 10.48% <ø> (ø)
application-templates-mortgage-certificate 44.39% <ø> (ø)
application-templates-parental-leave 28.38% <ø> (ø)
application-types 6.75% <ø> (ø)
application-ui-components 1.42% <ø> (ø)
application-ui-shell 21.28% <ø> (ø)
auth-admin-web 2.43% <ø> (ø)
auth-nest-tools 30.44% <ø> (ø)
auth-react 22.95% <ø> (ø)
auth-shared 75.00% <ø> (ø)
clients-charge-fjs-v2 27.54% <ø> (ø)
clients-driving-license 41.02% <ø> (ø)
clients-driving-license-book 44.14% <ø> (ø)
clients-financial-statements-inao 49.47% <ø> (ø)
clients-license-client 1.83% <ø> (ø)
clients-middlewares 72.69% <ø> (-0.35%) ⬇️
clients-regulations 42.97% <ø> (ø)
clients-rsk-personal-tax-return 38.00% <ø> (ø)
clients-smartsolutions 12.77% <ø> (ø)
clients-zendesk 55.65% <ø> (ø)
cms 0.43% <ø> (ø)
cms-translations 39.76% <ø> (ø)
content-search-index-manager 95.65% <ø> (ø)
content-search-toolkit 8.50% <ø> (ø)
contentful-apps 6.05% <ø> (ø)
dokobit-signing 62.67% <ø> (ø)
download-service 44.64% <ø> (ø)
email-service 60.49% <ø> (ø)
feature-flags 90.55% <ø> (ø)
file-storage 52.77% <ø> (ø)
financial-aid-backend 56.45% <ø> (ø)
financial-aid-shared 19.03% <ø> (ø)
icelandic-names-registry-backend 54.36% <ø> (ø)
infra-nest-server 48.33% <ø> (ø)
infra-tracing 43.24% <ø> (ø)
island-ui-core 28.76% <ø> (ø)
judicial-system-api 18.21% <ø> (ø)
judicial-system-audit-trail 68.36% <ø> (ø)
judicial-system-backend 56.22% <ø> (ø)
judicial-system-formatters 80.31% <ø> (ø)
judicial-system-message 66.66% <ø> (ø)
judicial-system-message-handler 47.43% <ø> (ø)
judicial-system-scheduler 69.19% <ø> (ø)
judicial-system-types 50.88% <ø> (ø)
license-api 42.99% <ø> (ø)
localization 10.15% <ø> (ø)
nest-audit 68.20% <ø> (ø)
nest-config 77.94% <ø> (ø)
nest-problem 46.19% <ø> (ø)
nest-sequelize 94.44% <ø> (ø)
nest-swagger 51.71% <ø> (ø)
nova-sms 62.00% <ø> (ø)
portals-admin-regulations-admin 2.02% <ø> (ø)
portals-core 16.22% <ø> (ø)
reference-backend 50.31% <ø> (ø)
regulations 16.78% <ø> (ø)
residence-history 85.00% <ø> (ø)
services-auth-admin-api 51.10% <ø> (ø)
services-auth-delegation-api 61.47% <ø> (ø)
services-auth-ids-api 53.88% <ø> ( 0.02%) ⬆️
services-auth-personal-representative 48.12% <ø> (ø)
services-auth-personal-representative-public 44.02% <ø> (ø)
services-auth-public-api 51.98% <ø> (-0.02%) ⬇️
services-documents 61.09% <ø> (ø)
services-user-notification 48.04% <ø> ( 0.02%) ⬆️
services-user-profile 62.35% <ø> (ø)
shared-form-fields 31.79% <ø> (ø)
shared-mocking 64.62% <ø> (ø)
shared-pii 92.85% <ø> (ø)
shared-utils 29.87% <ø> (ø)
skilavottord-ws 23.57% <ø> (ø)
web 1.88% <ø> (ø)

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

see 2 files 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 13c823b...7385730. Read the comment docs.

Copy link
Member

@baering baering left a comment

Choose a reason for hiding this comment

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

Should the mock not belong where the client code is located?

And as I said in a previous comment we should try to use the types we have as much as possible. Take a look at this reference for example.

Here there is a function imported from the gen folder to generate the JSON to return.

import { EinstaklingsupplysingarToJSON } from '@island.is/clients/national-registry-v2'

This way if there is ever a change in the clientConfig.json the mock file would not go out of sync and the response will always be correct.

@norda-gunni
Copy link
Contributor Author

norda-gunni commented Aug 6, 2024

@baering

Should the mock not belong where the client code is located?

Well the thinking was that this might function as a sort of central repo for not just the national registry v2 client but other clients as well as that might aid discoverability. However should the core team feel that the capture files belong with the clients themselves that is not a problem.

And as I said in a previous comment we should try to use the types we have as much as possible. Take a look at this reference for example.

The thing is that the json file included here is a generated file so types don't really apply. Should the types change, the capture need simply be re-generated. I did however change the readme to hopefully make the fact that the json files are generated more clear.

@norda-gunni norda-gunni added the automerge Merge this PR as soon as all checks pass label Aug 27, 2024
@kodiakhq kodiakhq bot merged commit 2627a5e into main Aug 27, 2024
254 checks passed
@kodiakhq kodiakhq bot deleted the devex/nat-reg-mocking branch August 27, 2024 15:32
HjorturJ pushed a commit that referenced this pull request Aug 27, 2024
* Add mockoon cli and national registry mock json

* Add readme

* Fix yarn.lock conflict

* Roll back to main version of package.json and yarn.lock

* Re-add mockoon cli after revert to main version

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Re-adding lost yarn script

* Remove mockoon cli

* yarn lock

* Re-add script

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Fix rabbit fix

* Rolled package.json and yarn.lock back to main and addressed pr comments

* Another revert attempt

* chore: nx format:write update dirty files

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Add clarity to radme and rename capture file to clarify that it is for version 2 only.

* More clarification in readme

* Add mockoon CLI as dev dependency

* Move and rename data file for added clarity

* chore: nx format:write update dirty files

---------

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: andes-it <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>
thoreyjona pushed a commit that referenced this pull request Aug 28, 2024
* Add mockoon cli and national registry mock json

* Add readme

* Fix yarn.lock conflict

* Roll back to main version of package.json and yarn.lock

* Re-add mockoon cli after revert to main version

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Re-adding lost yarn script

* Remove mockoon cli

* yarn lock

* Re-add script

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Fix rabbit fix

* Rolled package.json and yarn.lock back to main and addressed pr comments

* Another revert attempt

* chore: nx format:write update dirty files

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Add clarity to radme and rename capture file to clarify that it is for version 2 only.

* More clarification in readme

* Add mockoon CLI as dev dependency

* Move and rename data file for added clarity

* chore: nx format:write update dirty files

---------

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: andes-it <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>
kodiakhq bot added a commit that referenced this pull request Aug 29, 2024
…15794)

* Adding bearer auth to appropriate controllers

* Reverting changes in non application-system code

* fix(application-ir): Executor Quantifier (#15790)

* fix(service-portal): Finance mobile padding (#15640)

Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* chore(occupational-licenses): Display license number in health directorate licenses (#15775)

* chore: init

* chore: fix version

* chore: remove console.log

* chore: revert some

---------

Co-authored-by: Þorkell Máni Þorkelsson <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* feat(contentful-entry-tagger): Use team name as entry tagging substitute (#15791)

Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* feat(devex): Local mocking of calls to the national registry (#15523)

* Add mockoon cli and national registry mock json

* Add readme

* Fix yarn.lock conflict

* Roll back to main version of package.json and yarn.lock

* Re-add mockoon cli after revert to main version

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Re-adding lost yarn script

* Remove mockoon cli

* yarn lock

* Re-add script

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Fix rabbit fix

* Rolled package.json and yarn.lock back to main and addressed pr comments

* Another revert attempt

* chore: nx format:write update dirty files

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Add clarity to radme and rename capture file to clarify that it is for version 2 only.

* More clarification in readme

* Add mockoon CLI as dev dependency

* Move and rename data file for added clarity

* chore: nx format:write update dirty files

---------

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: andes-it <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* fix: url printing in ci (#15788)

* force deploy

* updated yargs

* reverted safe vars

* debugging comment message

* testing again

* removed debug printing

* removed debug printing

* checking file contents

* testing docker run script in CI

* removed logger debug and changed logger

* removed deploy

---------

Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* feat(web): Profession rights list - Display national id field in table view (#15792)

* Add national id field

* Cast as any

* Ignore type warnings due to pipeline not being up to date with types

---------

Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* Adding bearer auth to payment status endpoint

* Removing unneeded auth param from request, now done automatically

* chore: nx format:write update dirty files

---------

Co-authored-by: kksteini <77672665 [email protected]>
Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: Þorkell Máni Þorkelsson <[email protected]>
Co-authored-by: Þorkell Máni Þorkelsson <[email protected]>
Co-authored-by: Rúnar Vestmann <43557895 [email protected]>
Co-authored-by: norda-gunni <161026627 [email protected]>
Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: andes-it <[email protected]>
Co-authored-by: brynjarorng <[email protected]>
jonnigs pushed a commit that referenced this pull request Sep 2, 2024
* Add mockoon cli and national registry mock json

* Add readme

* Fix yarn.lock conflict

* Roll back to main version of package.json and yarn.lock

* Re-add mockoon cli after revert to main version

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Re-adding lost yarn script

* Remove mockoon cli

* yarn lock

* Re-add script

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Fix rabbit fix

* Rolled package.json and yarn.lock back to main and addressed pr comments

* Another revert attempt

* chore: nx format:write update dirty files

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Add clarity to radme and rename capture file to clarify that it is for version 2 only.

* More clarification in readme

* Add mockoon CLI as dev dependency

* Move and rename data file for added clarity

* chore: nx format:write update dirty files

---------

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: andes-it <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>
jonnigs pushed a commit that referenced this pull request Sep 2, 2024
…15794)

* Adding bearer auth to appropriate controllers

* Reverting changes in non application-system code

* fix(application-ir): Executor Quantifier (#15790)

* fix(service-portal): Finance mobile padding (#15640)

Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* chore(occupational-licenses): Display license number in health directorate licenses (#15775)

* chore: init

* chore: fix version

* chore: remove console.log

* chore: revert some

---------

Co-authored-by: Þorkell Máni Þorkelsson <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* feat(contentful-entry-tagger): Use team name as entry tagging substitute (#15791)

Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* feat(devex): Local mocking of calls to the national registry (#15523)

* Add mockoon cli and national registry mock json

* Add readme

* Fix yarn.lock conflict

* Roll back to main version of package.json and yarn.lock

* Re-add mockoon cli after revert to main version

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Re-adding lost yarn script

* Remove mockoon cli

* yarn lock

* Re-add script

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Fix rabbit fix

* Rolled package.json and yarn.lock back to main and addressed pr comments

* Another revert attempt

* chore: nx format:write update dirty files

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Update mocks/README.md

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>

* Add clarity to radme and rename capture file to clarify that it is for version 2 only.

* More clarification in readme

* Add mockoon CLI as dev dependency

* Move and rename data file for added clarity

* chore: nx format:write update dirty files

---------

Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: andes-it <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* fix: url printing in ci (#15788)

* force deploy

* updated yargs

* reverted safe vars

* debugging comment message

* testing again

* removed debug printing

* removed debug printing

* checking file contents

* testing docker run script in CI

* removed logger debug and changed logger

* removed deploy

---------

Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* feat(web): Profession rights list - Display national id field in table view (#15792)

* Add national id field

* Cast as any

* Ignore type warnings due to pipeline not being up to date with types

---------

Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>

* Adding bearer auth to payment status endpoint

* Removing unneeded auth param from request, now done automatically

* chore: nx format:write update dirty files

---------

Co-authored-by: kksteini <77672665 [email protected]>
Co-authored-by: Ásdís Erna Guðmundsdóttir <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102 kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: Þorkell Máni Þorkelsson <[email protected]>
Co-authored-by: Þorkell Máni Þorkelsson <[email protected]>
Co-authored-by: Rúnar Vestmann <43557895 [email protected]>
Co-authored-by: norda-gunni <161026627 [email protected]>
Co-authored-by: coderabbitai[bot] <136622811 coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: andes-it <[email protected]>
Co-authored-by: brynjarorng <[email protected]>
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.

3 participants