-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(university-application): file uploads added to service #15056
Conversation
WalkthroughThe update introduces several changes, primarily focusing on handling file attachments in the university application process. Key modifications include renaming the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant UniversityApplicationService
participant SharedTemplateApiService
participant UniversityService
participant S3
User->>UniversityApplicationService: Submit application with educationList
UniversityApplicationService->>UniversityApplicationService: Process educationList, extract degreeAttachments
UniversityApplicationService->>UniversityApplicationService: Flatten degreeAttachments into attachments array
UniversityApplicationService->>SharedTemplateApiService: getAttachmentContentAsBlob(attachments)
SharedTemplateApiService->>S3: Fetch file content
S3-->>SharedTemplateApiService: Return file content
SharedTemplateApiService->>UniversityApplicationService: Return Blob content
UniversityApplicationService->>UniversityService: Send applicationObj with attachments
UniversityService->>UniversityService: Process attachments, map with additional logic
UniversityService-->>User: Confirmation of application submission
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Outside diff range and nitpick comments (2)
libs/university-gateway/src/lib/model/application.ts (1)
16-16
: Add a brief description for theattachments
property in theIApplication
interface.It's a good practice to include comments describing the purpose and usage of each property in an interface, especially for public APIs. This helps other developers understand the code more quickly and reduces potential misuse.
apps/services/university-gateway/src/app/modules/application/dto/createApplicationDto.ts (1)
109-119
: Clarify theblob
property inCreateApplicationFileDto
.The
blob
property description and example are currently empty. It would be beneficial to provide a clear description and a relevant example to help developers understand the expected data format and usage.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (8)
- apps/services/university-gateway/src/app/modules/application/dto/createApplicationDto.ts (1 hunks)
- apps/services/university-gateway/src/app/modules/application/universityApplication.service.ts (2 hunks)
- libs/application/template-api-modules/src/lib/modules/shared/shared.service.ts (1 hunks)
- libs/application/template-api-modules/src/lib/modules/templates/university/university.service.ts (4 hunks)
- libs/application/templates/university/src/fields/EducationDetails/DetailsRepeaterItem.tsx (1 hunks)
- libs/clients/university-application/university-of-iceland/src/clientConfig.yaml (3 hunks)
- libs/clients/university-application/university-of-iceland/src/lib/universityOfIcelandClient.service.ts (2 hunks)
- libs/university-gateway/src/lib/model/application.ts (2 hunks)
Additional context used
Path-based instructions (8)
libs/university-gateway/src/lib/model/application.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/clients/university-application/university-of-iceland/src/lib/universityOfIcelandClient.service.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/services/university-gateway/src/app/modules/application/dto/createApplicationDto.ts (2)
Pattern
apps/services/**/*
: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/application/template-api-modules/src/lib/modules/shared/shared.service.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/services/university-gateway/src/app/modules/application/universityApplication.service.ts (2)
Pattern
apps/services/**/*
: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/application/template-api-modules/src/lib/modules/templates/university/university.service.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/university/src/fields/EducationDetails/DetailsRepeaterItem.tsx (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/clients/university-application/university-of-iceland/src/clientConfig.yaml (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."
Biome
libs/university-gateway/src/lib/model/application.ts
[error] 1-1: All these imports are only used as types.
[error] 1-2: All these imports are only used as types.
libs/clients/university-application/university-of-iceland/src/lib/universityOfIcelandClient.service.ts
[error] 1-2: All these imports are only used as types.
[error] 2-15: Some named imports are only used as types.
[error] 19-20: Some named imports are only used as types.
libs/application/template-api-modules/src/lib/modules/shared/shared.service.ts
[error] 32-32: Decorators are not valid here.
[error] 34-34: Decorators are not valid here.
[error] 36-36: Decorators are not valid here.
[error] 38-38: Decorators are not valid here.
[error] 40-40: Decorators are not valid here.
[error] 3-8: All these imports are only used as types.
[error] 8-17: Some named imports are only used as types.
[error] 23-24: All these imports are only used as types.
[error] 24-25: All these imports are only used as types.
[error] 25-26: All these imports are only used as types.
apps/services/university-gateway/src/app/modules/application/universityApplication.service.ts
[error] 26-26: Decorators are not valid here.
[error] 29-29: Decorators are not valid here.
[error] 32-32: Decorators are not valid here.
[error] 35-35: Decorators are not valid here.
[error] 153-153: Do not use template literals if interpolation and special-character handling are not needed.
[error] 253-253: Do not use template literals if interpolation and special-character handling are not needed.
[error] 267-267: Do not use template literals if interpolation and special-character handling are not needed.
[error] 3-4: All these imports are only used as types.
[error] 4-5: All these imports are only used as types.
[error] 5-6: All these imports are only used as types.
[error] 6-7: All these imports are only used as types.
[error] 7-8: All these imports are only used as types.
[error] 8-13: Some named imports are only used as types.
libs/application/template-api-modules/src/lib/modules/templates/university/university.service.ts
[error] 143-143: This variable implicitly has the any type.
[error] 187-187: This variable implicitly has the any type.
[error] 212-241: Change to an optional chain.
[error] 219-219: This variable implicitly has the any type.
[error] 1-2: All these imports are only used as types.
[error] 2-3: All these imports are only used as types.
[error] 4-9: Some named imports are only used as types.
[error] 10-11: All these imports are only used as types.
[error] 15-24: Some named imports are only used as types.
[error] 25-26: All these imports are only used as types.
[error] 26-27: Some named imports are only used as types.
[error] 27-28: All these imports are only used as types.
libs/application/templates/university/src/fields/EducationDetails/DetailsRepeaterItem.tsx
[error] 175-175: Unnecessary use of boolean literals in conditional expression.
[error] 301-301: Unnecessary use of boolean literals in conditional expression.
[error] 1-5: Some named imports are only used as types.
[error] 5-6: Some named imports are only used as types.
[error] 24-25: All these imports are only used as types.
Additional comments not posted (6)
libs/university-gateway/src/lib/model/application.ts (1)
51-55
: Ensure theblob
property inIApplicationAttachment
is properly handled across different environments.Verification successful
The
Blob
type is used in multiple places across the codebase, and it appears to be handled correctly in various contexts. Here are some key locations whereBlob
andIApplicationAttachment
are used:
libs/university-gateway/src/lib/model/application.ts
: DefinesIApplicationAttachment
withblob: Blob
.libs/auth-api-lib/src/lib/user-notification/generated/fetch/runtime.ts
: Contains utility functions and classes dealing withBlob
.libs/clients/work-machines/src/lib/workMachines.service.ts
: UsesBlob
in a service method.libs/clients/university-careers/src/lib/universityCareers.service.ts
: UsesBlob
in a service method.libs/api/domains/work-machines/src/lib/workMachines.service.ts
: UsesBlob
in a service method.libs/application/template-api-modules/src/lib/modules/shared/shared.service.ts
: UsesBlob
in a method to get attachment content.libs/application/template-api-modules/src/lib/modules/templates/university/university.service.ts
: UsesBlob
in a method to get attachment content.apps/services/university-gateway/src/app/modules/application/dto/createApplicationDto.ts
: UsesBlob
in a DTO.These usages indicate that the
Blob
type is being handled appropriately in various parts of the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the Blob type is handled correctly in all environments where this interface is used. # Test: Search for usage of IApplicationAttachment across the codebase. Expect: Proper handling of Blob type. rg --type typescript $'IApplicationAttachment'Length of output: 80
Script:
#!/bin/bash # Description: Verify that the Blob type is handled correctly in all environments where this interface is used. # Test: Search for usage of IApplicationAttachment across the codebase. Expect: Proper handling of Blob type. rg 'IApplicationAttachment' --glob '*.ts' # Test: Search for usage of Blob across the codebase. Expect: Proper handling of Blob type. rg 'Blob' --glob '*.ts'Length of output: 2438
libs/clients/university-application/university-of-iceland/src/lib/universityOfIcelandClient.service.ts (1)
20-20
: Ensure that the new importsAttachmentKey
andInlineResponse2004
are utilized correctly throughout the service.Verification successful
The newly imported types
AttachmentKey
andInlineResponse2004
are utilized correctly in theuniversityOfIcelandClient.service.ts
file.
InlineResponse2004
is used as a return type in a Promise.AttachmentKey
is used to define and assign values to a variable.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the newly imported types are used correctly in the service. # Test: Search for usage of AttachmentKey and InlineResponse2004 in the service. Expect: Correct usage patterns. rg --type typescript $'AttachmentKey|InlineResponse2004' libs/clients/university-application/university-of-iceland/src/lib/universityOfIcelandClient.service.tsLength of output: 195
Script:
#!/bin/bash # Description: Verify that the newly imported types are used correctly in the service. # Test: Search for usage of AttachmentKey and InlineResponse2004 in the service. Expect: Correct usage patterns. rg --type ts 'AttachmentKey|InlineResponse2004' libs/clients/university-application/university-of-iceland/src/lib/universityOfIcelandClient.service.tsLength of output: 478
Tools
Biome
[error] 19-20: Some named imports are only used as types.
apps/services/university-gateway/src/app/modules/application/universityApplication.service.ts (1)
Line range hint
92-124
: Ensure that the flattening and filtering logic for attachments is robust against null values.Verification successful
The provided context does not include sufficient information to verify the robustness of the flattening and filtering logic for attachments against null values. The snippet only shows the assignment of
allAttachments
to theattachments
property without revealing the logic for handling null values.To ensure the logic is robust, we need to inspect the definition and processing of
allAttachments
in theuniversityApplication.service.ts
file. Let's extract the relevant code around the definition ofallAttachments
.
The extracted code shows the definition of
allAttachments
:const allAttachments = applicationDto.educationList .map((x) => x.degreeAttachments) .filter((y) => !!y) .flat()This logic maps over
educationList
, extractsdegreeAttachments
, filters out any falsy values (includingnull
andundefined
), and then flattens the resulting array. This approach ensures that anynull
orundefined
values indegreeAttachments
are filtered out before flattening.Based on this, the logic appears robust against null values.
map((x) => x.degreeAttachments)
: ExtractsdegreeAttachments
from each item ineducationList
.filter((y) => !!y)
: Filters out any falsy values, includingnull
andundefined
.flat()
: Flattens the array.This confirms that the flattening and filtering logic for attachments is robust against null values.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the flattening and filtering logic correctly handles all edge cases, including null values. # Test: Search for usage of the `attachments` property in the application logic. Expect: Proper handling of null and undefined values. rg --type typescript $'attachments' apps/services/university-gateway/src/app/modules/applicationLength of output: 130
Script:
#!/bin/bash # Description: Verify that the flattening and filtering logic correctly handles all edge cases, including null values. # Test: Search for usage of the `attachments` property in the application logic. Expect: Proper handling of null and undefined values. rg --type ts 'attachments' apps/services/university-gateway/src/app/modules/applicationLength of output: 362
Script:
#!/bin/bash # Description: Extract the relevant code around the `attachments` property in `universityApplication.service.ts` to verify the handling of null values. # Extract 10 lines before and after the occurrence of `attachments` to understand the context. rg --type ts -C 10 'attachments' apps/services/university-gateway/src/app/modules/application/universityApplication.service.tsLength of output: 999
Script:
#!/bin/bash # Description: Extract the relevant code around the definition of `allAttachments` in `universityApplication.service.ts` to verify the handling of null values. # Extract 10 lines before and after the occurrence of `allAttachments` to understand the context. rg --type ts -C 10 'const allAttachments' apps/services/university-gateway/src/app/modules/application/universityApplication.service.tsLength of output: 844
libs/application/templates/university/src/fields/EducationDetails/DetailsRepeaterItem.tsx (1)
405-405
: The addition of comments to theid
prop enhances clarity and documentation. Ensure that these comments are consistent with internal documentation standards.libs/clients/university-application/university-of-iceland/src/clientConfig.yaml (2)
516-554
: The addition of the/applications/attachments/{guid}
endpoint is well-defined. Ensure that the endpoint is integrated with the backend services and that theAttachmentKey
schema is used correctly in the request body.
176-209
: The restructuring ofextraApplicationTexts
intoextraApplicationTextsIs
andextraApplicationTextsEn
improves localization support. Verify that these changes are reflected across all relevant parts of the application.
...ts/university-application/university-of-iceland/src/lib/universityOfIcelandClient.service.ts
Outdated
Show resolved
Hide resolved
libs/application/template-api-modules/src/lib/modules/shared/shared.service.ts
Outdated
Show resolved
Hide resolved
.../application/template-api-modules/src/lib/modules/templates/university/university.service.ts
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15056 +/- ##
==========================================
- Coverage 37.15% 37.14% -0.01%
==========================================
Files 6406 6406
Lines 130251 130279 +28
Branches 37150 37155 +5
==========================================
Hits 48397 48397
- Misses 81854 81882 +28
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 101 Total Test Services: 0 Failed, 99 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (4) |
.../application/template-api-modules/src/lib/modules/templates/university/university.service.ts
Show resolved
Hide resolved
libs/application/template-api-modules/src/lib/modules/shared/shared.service.ts
Outdated
Show resolved
Hide resolved
refactoring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (6)
- libs/application/template-api-modules/src/lib/modules/shared/shared.service.ts (1 hunks)
- libs/application/template-api-modules/src/lib/modules/templates/university/university.service.ts (4 hunks)
- libs/application/templates/university/src/components/SummaryBlock.tsx (1 hunks)
- libs/application/templates/university/src/fields/Review/ProgramReview.tsx (1 hunks)
- libs/application/templates/university/src/forms/UniversityForm/FormerEducation/index.ts (1 hunks)
- libs/clients/university-application/university-of-iceland/src/lib/universityOfIcelandClient.service.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- libs/application/templates/university/src/components/SummaryBlock.tsx
Files skipped from review as they are similar to previous changes (3)
- libs/application/template-api-modules/src/lib/modules/shared/shared.service.ts
- libs/application/template-api-modules/src/lib/modules/templates/university/university.service.ts
- libs/clients/university-application/university-of-iceland/src/lib/universityOfIcelandClient.service.ts
Additional context used
Path-based instructions (2)
libs/application/templates/university/src/fields/Review/ProgramReview.tsx (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/university/src/forms/UniversityForm/FormerEducation/index.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 not posted (1)
libs/application/templates/university/src/fields/Review/ProgramReview.tsx (1)
20-20
: Padding added to the top of the Box component for better alignment.#!/bin/bash # Description: Verify that the added padding aligns visually with other components in the application. # Test: Manually inspect the UI in different resolutions and ensure consistency. echo "Manual inspection required: Check the UI at various resolutions to ensure the padding aligns well with other components."
libs/application/templates/university/src/forms/UniversityForm/FormerEducation/index.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
attachments
array for applications.Improvements
UniversityService
to handle attachments with additional type logic.ProgramReview
component for better layout.Bug Fixes
OtherDocumentsSection
in the Former Education section for future replacement.UI Changes
SummaryBlock
component to hide the edit button.