feat(form-system): Application #15444
Conversation
…into form-builder
…cript, removed all console.logs and removed all code that was commented out
…into form-builder
…n + script to generate graphql schemas
This commit adds the form system API setup to the uber-charts/islandis.ts file. This allows the form system service to be used in the application.
…rm-system-new-client-domain
…rm-system-new-client-domain
…rm-system-new-client-domain
…rm-system-new-client-domain
…rm-system-new-client-domain
…rm-system-new-client-domain
…rm-system-new-client-domain
…rm-system-new-client-domain
…land-is/island.is into form-system-new-client-domain
…rm-system-new-client-domain
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| - | - | X-API-Key Secret | fc7f69d | apps/form-builder/services/translationStation.tsx | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/judicial-system/api/src/app/modules/file/limitedAccessFile.controller.ts (2)
9-9: Remove unused import.The
Querydecorator is imported but not used anywhere in the controller.- Query,
23-23: Remove unused import.The
SubpoenaTypetype is imported but not used in the controller.-import type { SubpoenaType, User } from '@island.is/judicial-system/types' +import type { User } from '@island.is/judicial-system/types'
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.eslintrc.json(1 hunks)apps/api/infra/api.ts(2 hunks)apps/judicial-system/api/src/app/modules/file/file.controller.ts(2 hunks)apps/judicial-system/api/src/app/modules/file/limitedAccessFile.controller.ts(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/judicial-system/api/src/app/modules/file/file.controller.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/api/infra/api.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/judicial-system/api/src/app/modules/file/limitedAccessFile.controller.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 (2)
.eslintrc.json (1)
45-45: LGTM!
The addition of lib:form-system to the scope:portals-admin dependencies aligns with the PR objectives for the form-system project integration.
apps/api/infra/api.ts (1)
68-68: LGTM!
The addition of formSystemService parameter aligns with the form-system project integration.
| dev: 'http://localhost:3434', | ||
| staging: '', | ||
| prod: '', | ||
| }, | ||
| // FORM_SYSTEM_API_BASE_PATH: ref( | ||
| // (h) => `http://${h.svc(services.formSystemService)}`, | ||
| // ), |
There was a problem hiding this comment.
Review environment configuration values.
There are several concerns with the current configuration:
- Staging and prod environments have empty string values
- Development environment uses a hardcoded localhost URL
- There's a commented-out configuration that might be the intended approach using the service reference
Consider either:
- Providing proper URLs for staging and prod environments
- Using the service reference approach as shown in the commented code
- FORM_SYSTEM_API_BASE_PATH: {
- dev: 'http://localhost:3434',
- staging: '',
- prod: '',
- },
- // FORM_SYSTEM_API_BASE_PATH: ref(
- // (h) => `http://${h.svc(services.formSystemService)}`,
- // ),
+ FORM_SYSTEM_API_BASE_PATH: ref(
+ (h) => `http://${h.svc(services.formSystemService)}`,
+ ),Committable suggestion skipped: line range outside the PR's diff.
…ollo Client setup
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
libs/portals/form-system/graphql/src/lib/client.ts (2)
11-11: Consider configuring RetryLink with operation-specific retry policies.The default retry configuration might not be optimal for all operations. Consider customizing the retry logic:
-const retryLink = new RetryLink() +const retryLink = new RetryLink({ + delay: { + initial: 300, + max: 3000, + jitter: true + }, + attempts: { + max: 3, + retryIf: (error, operation) => { + // Retry on network errors but not on client errors + return !!error && operation.operationName !== 'specialOperation' + } + } +})
31-35: Consider configuring InMemoryCache for optimal performance.The default cache configuration might not be optimal for your use case. Consider customizing it:
return new ApolloClient({ link: ApolloLink.from([retryLink, errorLink, httpLink].filter(Boolean)), - cache: new InMemoryCache(), + cache: new InMemoryCache({ + typePolicies: { + Query: { + fields: { + // Example: Configure field-specific caching + forms: { + merge: true, + // Custom merge function if needed + } + } + } + }, + possibleTypes: { + // Add possible types for interfaces/unions + } + }), })
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
libs/portals/form-system/graphql/src/lib/client.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/portals/form-system/graphql/src/lib/client.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 (2)
libs/portals/form-system/graphql/src/lib/client.ts (2)
1-10: Well-structured imports with good tree-shaking practices!The imports are properly organized and use specific paths, which is great for tree-shaking. Good choice using
cross-fetchfor universal fetch support.
25-30: Well-implemented dynamic GraphQL endpoint configuration!The client initialization with dynamic URI construction based on operation name is a good approach. The function is properly typed and exported, adhering to TypeScript guidelines.
| const errorLink = onError(({ graphQLErrors, networkError }) => { | ||
| if (graphQLErrors) | ||
| graphQLErrors.map(({ message, locations, path, extensions }) => { | ||
| const problem = JSON.stringify(extensions?.problem, null, ' ') | ||
| console.log( | ||
| `[GraphQL error]: Message: ${message}, Path: ${path}, Problem: ${problem}`, | ||
| ) | ||
| }) | ||
|
|
||
| if (networkError) console.log(`[Network error]: ${networkError}`) | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling for production use.
While the error logging is comprehensive, using console.log in production isn't ideal. Consider these improvements:
const errorLink = onError(({ graphQLErrors, networkError }) => {
if (graphQLErrors)
graphQLErrors.map(({ message, locations, path, extensions }) => {
const problem = JSON.stringify(extensions?.problem, null, ' ')
- console.log(
- `[GraphQL error]: Message: ${message}, Path: ${path}, Problem: ${problem}`,
- )
+ // Use a proper error tracking service
+ errorTracker.captureException({
+ type: 'GraphQLError',
+ message,
+ path,
+ problem,
+ locations
+ })
})
- if (networkError) console.log(`[Network error]: ${networkError}`)
+ if (networkError) {
+ errorTracker.captureException({
+ type: 'NetworkError',
+ error: networkError
+ })
+ }
+
+ // Allow consuming code to handle errors
+ return forward(operation)
})Committable suggestion skipped: line range outside the PR's diff.
...
Checklist:
Summary by CodeRabbit
New Features
LimitedAccessFileControllerto support specific subpoena retrieval based on IDs.Improvements
FileControllerfor better readability.