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

Pull Request for changes made to repository after redeployment to new AWS account #17

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

Conversation

ymijsters
Copy link
Collaborator

@ymijsters ymijsters commented Nov 21, 2024

Also includes a bug fix where caching is not working properly, blocking the fileupload function. This issue is currently resolved, removing cache, and the caching is parked on the issue list.

Summary by CodeRabbit

  • New Features
    • Increased request payload limit allows uploads up to 25 MB.
    • Mobile authentication now points to an updated endpoint for enhanced mobile access.
  • Branding & UI
    • Email notifications now reflect LNCO.ai branding.
    • Extended token expiration provides longer, smoother sessions.
  • Documentation
    • Comprehensive user guides and API documentation have been reformatted for improved clarity.
  • Internationalization
    • Language translations have been updated to deliver a more consistent localized experience.

* fix: update graasp-dev.json

* fix: update deploy-dev.yml add h5p-storage-root-path

* fix: update server.ts

* fix: add some console logs to find issue

* fix: add a lot more logs

* fix: remove etherpad from dependency build

* fix: remove more Etherpad content

* fix: one more etherpad thing removed

* fix: one more

* fix: remove h5p integration in import-export service

* fix: update container.ts to remove h5p

* fix: remove etherpad and h5p from api

* fix: remove ws component

* fix: use prettier --write on everything

* feat: remove websocket + redis, etherpad, h5p and meilisearch from initialization

* feat: take out etherpad + h5p + meilisearch, keep redis

* feat: completely-remove-etherpad-and-h5p-code

* feat: removed test for search service using meilisearch

* fix: reenabled commented items from testUtils

* fix: readd published item content to DataSource.ts

* fix: remove h5p and etherpad tests from import service

* feat: remove meilisearch code and test

* fix: remove meilisearch from mockSetup.ts

* fix: remove one more H5P test

* fix: remove one more meilisearch test line

* fix: removed previously commented out code and re-enabled some husky features
* fix: update graasp-dev.json

* fix: update deploy-dev.yml add h5p-storage-root-path

* fix: update server.ts

* fix: add some console logs to find issue

* fix: add a lot more logs

* fix: remove etherpad from dependency build

* fix: remove more Etherpad content

* fix: one more etherpad thing removed

* fix: one more

* fix: remove h5p integration in import-export service

* fix: update container.ts to remove h5p

* fix: remove etherpad and h5p from api

* fix: remove ws component

* fix: use prettier --write on everything

* feat: remove websocket + redis, etherpad, h5p and meilisearch from initialization

* feat: take out etherpad + h5p + meilisearch, keep redis

* feat: completely-remove-etherpad-and-h5p-code

* feat: removed test for search service using meilisearch

* fix: reenabled commented items from testUtils

* fix: readd published item content to DataSource.ts

* fix: remove h5p and etherpad tests from import service

* feat: remove meilisearch code and test

* fix: remove meilisearch from mockSetup.ts

* fix: remove one more H5P test

* fix: remove one more meilisearch test line

* fix: removed previously commented out code and re-enabled some husky features
* fix: update graasp-dev.json

* fix: update deploy-dev.yml add h5p-storage-root-path

* fix: update server.ts

* fix: add some console logs to find issue

* fix: add a lot more logs

* fix: remove etherpad from dependency build

* fix: remove more Etherpad content

* fix: one more etherpad thing removed

* fix: one more

* fix: remove h5p integration in import-export service

* fix: update container.ts to remove h5p

* fix: remove etherpad and h5p from api

* fix: remove ws component

* fix: use prettier --write on everything

* feat: remove websocket + redis, etherpad, h5p and meilisearch from initialization

* feat: take out etherpad + h5p + meilisearch, keep redis

* feat: completely-remove-etherpad-and-h5p-code

* feat: removed test for search service using meilisearch

* fix: reenabled commented items from testUtils

* fix: readd published item content to DataSource.ts

* fix: remove h5p and etherpad tests from import service

* feat: remove meilisearch code and test

* fix: remove meilisearch from mockSetup.ts

* fix: remove one more H5P test

* fix: remove one more meilisearch test line

* fix: removed previously commented out code and re-enabled some husky features
* fix: update graasp-dev.json

* fix: update deploy-dev.yml add h5p-storage-root-path

* fix: update server.ts

* fix: add some console logs to find issue

* fix: add a lot more logs

* fix: remove etherpad from dependency build

* fix: remove more Etherpad content

* fix: one more etherpad thing removed

* fix: one more

* fix: remove h5p integration in import-export service

* fix: update container.ts to remove h5p

* fix: remove etherpad and h5p from api

* fix: remove ws component

* fix: use prettier --write on everything

* feat: remove websocket + redis, etherpad, h5p and meilisearch from initialization

* feat: take out etherpad + h5p + meilisearch, keep redis

* feat: completely-remove-etherpad-and-h5p-code

* feat: removed test for search service using meilisearch

* fix: reenabled commented items from testUtils

* fix: readd published item content to DataSource.ts

* fix: remove h5p and etherpad tests from import service

* feat: remove meilisearch code and test

* fix: remove meilisearch from mockSetup.ts

* fix: remove one more H5P test

* fix: remove one more meilisearch test line

* fix: removed previously commented out code and re-enabled some husky features
* fix: update graasp-dev.json

* fix: update deploy-dev.yml add h5p-storage-root-path

* fix: update server.ts

* fix: add some console logs to find issue

* fix: add a lot more logs

* fix: remove etherpad from dependency build

* fix: remove more Etherpad content

* fix: one more etherpad thing removed

* fix: one more

* fix: remove h5p integration in import-export service

* fix: update container.ts to remove h5p

* fix: remove etherpad and h5p from api

* fix: remove ws component

* fix: use prettier --write on everything

* feat: remove websocket + redis, etherpad, h5p and meilisearch from initialization

* feat: take out etherpad + h5p + meilisearch, keep redis

* feat: completely-remove-etherpad-and-h5p-code

* feat: removed test for search service using meilisearch

* fix: reenabled commented items from testUtils

* fix: readd published item content to DataSource.ts

* fix: remove h5p and etherpad tests from import service

* feat: remove meilisearch code and test

* fix: remove meilisearch from mockSetup.ts

* fix: remove one more H5P test

* fix: remove one more meilisearch test line

* fix: removed previously commented out code and re-enabled some husky features
Copy link

@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: 14

🧹 Nitpick comments (23)
src/fastify.ts (1)

8-8: Consider enhancing logging configuration

With the removal of detailed logging configuration:

  1. Ensure proper logging is configured in your AWS environment (e.g., CloudWatch)
  2. Consider adding request ID tracking for better debugging
  3. Verify error reporting still works effectively with Sentry

Also applies to: 45-45

src/services/thumbnail/service.ts (1)

Line range hint 67-72: LGTM! Consider adding error handling

The new getFile method implementation looks good and aligns with the service's patterns. However, consider adding:

  1. Error handling for cases where the file doesn't exist
  2. Type validation for the size parameter against ThumbnailSizeFormat
 async getFile(actor: Actor, { id, size }: { size: string; id: string }) {
+  if (!Object.keys(ThumbnailSizeFormat).includes(size)) {
+    throw new Error(`Invalid thumbnail size: ${size}`);
+  }
   const result = await this._fileService.getFile(actor, {
     path: this.buildFilePath(id, size),
   });
+  if (!result) {
+    throw new Error(`Thumbnail not found for id: ${id}, size: ${size}`);
+  }
   return result;
 }
src/services/file/service.ts (1)

65-65: Cache operations have been disabled as a temporary fix.

The commented-out cache operations align with the PR objective to resolve file upload functionality issues. However, this should be treated as a temporary solution.

Consider:

  1. Adding a TODO comment to track the caching reimplementation
  2. Creating a feature flag to control caching behavior
  3. Implementing a more robust caching strategy that handles file upload edge cases

Also applies to: 121-121, 129-129

.github/workflows/test.yml (1)

99-100: Fix potential word splitting in shell commands.

The yarn cache directory commands should use quotes to prevent potential word splitting issues.

Apply this fix to both occurrences:

-echo "dir=$(yarn config get cacheFolder)" >> $GITHUB_OUTPUT
+echo "dir=\"$(yarn config get cacheFolder)\"" >> $GITHUB_OUTPUT

Also applies to: 191-192

🧰 Tools
🪛 actionlint (1.7.4)

99-99: shellcheck reported issue in this script: SC2086:info:1:46: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/deploy-stage.yml (1)

38-40: Consider using OIDC for AWS authentication.

Currently using long-lived AWS access keys. Consider using OpenID Connect (OIDC) with GitHub Actions for more secure, temporary credentials.

src/services/item/plugins/app/docs/api.md (2)

128-135: Document caching behavior for file uploads.

Given the PR's objective to fix caching issues with file uploads, consider adding a section about caching behavior and potential troubleshooting steps.

Would you like me to draft the additional documentation section about caching behavior?


208-215: Clarify file upload limitations and error scenarios.

The documentation should include:

  • Maximum file size limits
  • Supported file types
  • Common error scenarios and their resolutions
  • Caching behavior and invalidation
src/services/item/plugins/recycled/README.md (2)

15-15: Grammar: Fix possessive pronoun

Change "its" to "their" for consistency with the rest of the document.

- The user will see B in its trash.
+ The user will see B in their trash.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~15-~15: The determiner “the” seems more likely in this context.
Context: ...f A, recycled

The user will see B in its trash.

Admin Permission on child...

(AI_EN_LECTOR_REPLACEMENT_DETERMINER)


25-25: Documentation: Add storage cleanup details

The documentation mentions storage computation implications but doesn't specify how or when storage cleanup occurs for recycled items.

Consider adding a section about:

  • Storage cleanup policies for recycled items
  • How long items remain in trash before permanent deletion
  • How to manually trigger cleanup if needed
🧰 Tools
🪛 LanguageTool

[style] ~25-~25: ‘taken into account’ might be wordy. Consider a shorter alternative.
Context: ...t anymore nor remove it. This should be taken into account when computing the storage of the user....

(EN_WORDINESS_PREMIUM_TAKEN_INTO_ACCOUNT)

src/services/itemMembership/specifications.md (2)

5-5: Minor grammar improvement needed

Change "can not" to "cannot" for better readability and formal documentation style.

-A children membership can not be more restrictive
+A children membership cannot be more restrictive
🧰 Tools
🪛 LanguageTool

[style] ~5-~5: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...itance rules

  • A children membership can not be more restrictive (lower permission) ...

(CAN_NOT_PREMIUM)


5-26: Consider adding rationale for inheritance rules

While the rules and examples are clear, it would be helpful to add a brief explanation of why these inheritance rules exist (e.g., security implications, consistency guarantees). This would help developers better understand and remember the constraints.

🧰 Tools
🪛 LanguageTool

[style] ~5-~5: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...itance rules

  • A children membership can not be more restrictive (lower permission) ...

(CAN_NOT_PREMIUM)

docs/architecture.md (1)

9-9: Fix abbreviation formatting

Update abbreviations to follow standard technical writing conventions:

-The backend is structured using Fastify's plugins. Each plugin represents a feature (eg. member,
+The backend is structured using Fastify's plugins. Each plugin represents a feature (e.g., member,
🧰 Tools
🪛 LanguageTool

[uncategorized] ~9-~9: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...gins. Each plugin represents a feature (eg. member, item like, favorite, etc). Each...

(E_G)


[style] ~9-~9: In American English, abbreviations like “etc.” require a period.
Context: ...ature (eg. member, item like, favorite, etc). Each plugin is structured with one or...

(ETC_PERIOD)

src/services/item/plugins/invitation/specifications.md (3)

88-88: Fix typo in error reasons list

Correct the spelling of "permissions":

-- modification of existing persmissions on an item
+- modification of existing permissions on an item

81-89: Consider enhancing error documentation

The error reasons list would be more helpful with:

  1. HTTP status codes for each error scenario
  2. Example error response payloads
  3. Specific error codes or identifiers for client handling

This would help API consumers better handle error cases programmatically.


96-96: Fix typo in invitation handling description

Correct the spelling of "dropped":

-An email that is already invited on an item will be droped.
+An email that is already invited on an item will be dropped.
src/services/item/plugins/publication/README.md (1)

21-21: Fix typo in documentation

There's a typo in the sentence: "requiring the item to ve validated again" should be "requiring the item to be validated again"

src/services/websockets/API.md (2)

7-7: Fix grammar in documentation

Change "are placeholder" to "are placeholders" to correct the noun agreement.

🧰 Tools
🪛 LanguageTool

[grammar] ~7-~7: It’s possible that the countable noun ‘placeholder’ after the verb ‘are’ is an error. You may have meant to use the plural form, or an article may be missing.
Context: ...ween angle brackets ("<example>") are placeholder for dynamic values unless specified oth...

(BE_VB_OR_NN)


22-22: Fix grammar in documentation

Change "perform subscriptions changes requests" to "perform subscription change requests" for better clarity.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~22-~22: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...action field to perform subscriptions changes requests. Clients may send one of the f...

(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)

.github/workflows/deploy-dev.yml (2)

36-38: Address TODO comments regarding AWS credentials

The TODO comments indicate that AWS credentials should be added to environments. This should be addressed to ensure proper credential management.

Would you like me to help create a GitHub issue to track this task?


126-126: Consider explicit boolean value for JOB_SCHEDULING

The fallback to false using || might not work as expected in GitHub Actions. Consider making the value explicit in the environment variables.

- JOB_SCHEDULING=${{ vars.JOB_SCHEDULING || false }}
+ JOB_SCHEDULING=${{ vars.JOB_SCHEDULING }}
src/services/websockets/USAGE.md (1)

87-88: Add security considerations for websocket validation.

The documentation should include security best practices for websocket validation, such as:

  • Rate limiting
  • Payload size restrictions
  • Connection timeout handling
README.md (2)

8-8: Fix typo in configuration word.

There's a typo in the word "configurations".

Apply this diff:

-This repository contains the source code and confgurations for the Graasp Backend. Visit the Graasp Platform at [graasp.org](https://graasp.org)
+This repository contains the source code and configurations for the Graasp Backend. Visit the Graasp Platform at [graasp.org](https://graasp.org)

245-247: Fix markdown formatting for external link.

The Umami login documentation contains a bare URL. Use proper markdown link formatting.

Apply this diff:

-https://umami.is/docs/login
+[Umami Login Documentation](https://umami.is/docs/login)
🧰 Tools
🪛 Markdownlint (0.37.0)

245-245: null
Bare URL used

(MD034, no-bare-urls)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 671972b and 74ce71e.

⛔ Files ignored due to path filters (4)
  • src/services/item/plugins/invitation/test/fixtures/invalid-no-email.csv is excluded by !**/*.csv
  • src/services/item/plugins/invitation/test/fixtures/single-user.csv is excluded by !**/*.csv
  • src/services/item/plugins/invitation/test/fixtures/users-groups.csv is excluded by !**/*.csv
  • src/services/item/plugins/invitation/test/fixtures/users.csv is excluded by !**/*.csv
📒 Files selected for processing (80)
  • .aws/graasp-dev.json (4 hunks)
  • .aws/graasp-stage.json (1 hunks)
  • .devcontainer/Dockerfile (1 hunks)
  • .devcontainer/devcontainer.json (1 hunks)
  • .devcontainer/docker-compose.yml (1 hunks)
  • .devcontainer/localstack/init.sh (1 hunks)
  • .dockerignore (1 hunks)
  • .editorconfig (1 hunks)
  • .eslintignore (1 hunks)
  • .eslintrc (1 hunks)
  • .github/workflows/deploy-dev.yml (1 hunks)
  • .github/workflows/deploy-prod.yml (1 hunks)
  • .github/workflows/deploy-stage.yml (1 hunks)
  • .github/workflows/test.yml (1 hunks)
  • .gitignore (1 hunks)
  • .husky/.gitignore (1 hunks)
  • .husky/commit-msg (1 hunks)
  • .husky/install.mjs (1 hunks)
  • .husky/post-commit (1 hunks)
  • .husky/pre-commit (1 hunks)
  • .husky/pre-push (1 hunks)
  • .prettierignore (1 hunks)
  • .prettierrc (1 hunks)
  • .vscode/extensions.json (1 hunks)
  • .vscode/launch.json (1 hunks)
  • .vscode/tasks.json (1 hunks)
  • API Collections/Auth/Mobile/Auth.bru (1 hunks)
  • API Collections/Auth/Mobile/Refresh Token.bru (1 hunks)
  • API Collections/Auth/Mobile/Register.bru (1 hunks)
  • API Collections/Auth/Mobile/Sign In.bru (1 hunks)
  • API Collections/Auth/Reset Password/Ask Reset Password.bru (1 hunks)
  • API Collections/Auth/Reset Password/Set Password.bru (1 hunks)
  • API Collections/Auth/Web/Auth (magic link).bru (1 hunks)
  • API Collections/Auth/Web/Logout.bru (1 hunks)
  • API Collections/Auth/Web/Register.bru (1 hunks)
  • API Collections/Auth/Web/Sign In.bru (1 hunks)
  • API Collections/Invitations/All Item Invitations.bru (1 hunks)
  • API Collections/Invitations/Single Invitation.bru (1 hunks)
  • API Collections/Invitations/Upload Template users csv.bru (1 hunks)
  • API Collections/Invitations/Upload users csv.bru (1 hunks)
  • API Collections/Item/Accessible.bru (1 hunks)
  • API Collections/Item/Create Link Item.bru (1 hunks)
  • API Collections/Item/Get Item.bru (1 hunks)
  • API Collections/Item/Memberships/Delete membership.bru (1 hunks)
  • API Collections/Item/Memberships/Get Item memberships.bru (1 hunks)
  • API Collections/Item/Update Link Item.bru (1 hunks)
  • API Collections/Member/Current Member.bru (1 hunks)
  • API Collections/Member/Delete Current Member.bru (1 hunks)
  • API Collections/Member/Patch Current Member.bru (1 hunks)
  • API Collections/Zip/Import zip.bru (1 hunks)
  • API Collections/bruno.json (1 hunks)
  • API Collections/collection.bru (1 hunks)
  • API Collections/environments/Dev.bru (1 hunks)
  • API Collections/environments/Graasp Local.bru (1 hunks)
  • LICENSE (1 hunks)
  • README.md (1 hunks)
  • docker/Dockerfile (1 hunks)
  • docs/architecture.md (1 hunks)
  • src/fastify.ts (2 hunks)
  • src/plugins/mailer/langs/ar.json (1 hunks)
  • src/plugins/mailer/langs/de.json (1 hunks)
  • src/plugins/mailer/langs/en.json (1 hunks)
  • src/plugins/mailer/langs/es.json (1 hunks)
  • src/plugins/mailer/langs/fr.json (1 hunks)
  • src/plugins/mailer/langs/it.json (1 hunks)
  • src/plugins/mailer/layout.ts (1 hunks)
  • src/services/file/service.ts (2 hunks)
  • src/services/item/plugins/app/constants.ts (1 hunks)
  • src/services/item/plugins/app/docs/api.md (1 hunks)
  • src/services/item/plugins/file/index.ts (0 hunks)
  • src/services/item/plugins/invitation/specifications.md (1 hunks)
  • src/services/item/plugins/publication/README.md (1 hunks)
  • src/services/item/plugins/recycled/README.md (1 hunks)
  • src/services/item/plugins/thumbnail/service.ts (0 hunks)
  • src/services/itemMembership/specifications.md (1 hunks)
  • src/services/thumbnail/service.ts (1 hunks)
  • src/services/websockets/API.md (1 hunks)
  • src/services/websockets/USAGE.md (1 hunks)
  • src/utils/config.ts (1 hunks)
  • test/db/init.sql (1 hunks)
💤 Files with no reviewable changes (2)
  • src/services/item/plugins/thumbnail/service.ts
  • src/services/item/plugins/file/index.ts
✅ Files skipped from review due to trivial changes (54)
  • API Collections/Invitations/Upload Template users csv.bru
  • .husky/.gitignore
  • .prettierignore
  • .vscode/tasks.json
  • API Collections/Item/Update Link Item.bru
  • API Collections/Item/Create Link Item.bru
  • API Collections/Auth/Web/Sign In.bru
  • API Collections/environments/Dev.bru
  • API Collections/Auth/Web/Register.bru
  • API Collections/Member/Current Member.bru
  • .husky/install.mjs
  • API Collections/Item/Get Item.bru
  • .husky/pre-commit
  • src/plugins/mailer/langs/es.json
  • API Collections/Item/Accessible.bru
  • API Collections/Auth/Mobile/Register.bru
  • test/db/init.sql
  • API Collections/Auth/Web/Logout.bru
  • .vscode/extensions.json
  • API Collections/Auth/Reset Password/Ask Reset Password.bru
  • API Collections/Member/Delete Current Member.bru
  • API Collections/bruno.json
  • API Collections/Auth/Mobile/Sign In.bru
  • API Collections/collection.bru
  • .husky/commit-msg
  • .vscode/launch.json
  • .husky/pre-push
  • API Collections/Member/Patch Current Member.bru
  • API Collections/Invitations/All Item Invitations.bru
  • .devcontainer/devcontainer.json
  • .editorconfig
  • API Collections/Invitations/Single Invitation.bru
  • .dockerignore
  • .eslintignore
  • API Collections/Invitations/Upload users csv.bru
  • API Collections/Item/Memberships/Delete membership.bru
  • API Collections/Item/Memberships/Get Item memberships.bru
  • src/plugins/mailer/langs/ar.json
  • API Collections/Zip/Import zip.bru
  • API Collections/environments/Graasp Local.bru
  • .prettierrc
  • API Collections/Auth/Mobile/Auth.bru
  • API Collections/Auth/Mobile/Refresh Token.bru
  • .aws/graasp-stage.json
  • LICENSE
  • .devcontainer/Dockerfile
  • .gitignore
  • docker/Dockerfile
  • src/plugins/mailer/layout.ts
  • API Collections/Auth/Reset Password/Set Password.bru
  • src/plugins/mailer/langs/de.json
  • src/plugins/mailer/langs/it.json
  • .eslintrc
  • .husky/post-commit
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test.yml

99-99: shellcheck reported issue in this script: SC2086:info:1:46: Double quote to prevent globbing and word splitting

(shellcheck)


191-191: shellcheck reported issue in this script: SC2086:info:1:46: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/deploy-stage.yml

52-52: shellcheck reported issue in this script: SC2086:info:1:41: Double quote to prevent globbing and word splitting

(shellcheck)


59-59: shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting

(shellcheck)


59-59: shellcheck reported issue in this script: SC2086:info:2:13: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/deploy-dev.yml

50-50: shellcheck reported issue in this script: SC2086:info:1:41: Double quote to prevent globbing and word splitting

(shellcheck)


57-57: shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting

(shellcheck)


57-57: shellcheck reported issue in this script: SC2086:info:2:13: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/deploy-prod.yml

52-52: shellcheck reported issue in this script: SC2086:info:1:41: Double quote to prevent globbing and word splitting

(shellcheck)


59-59: shellcheck reported issue in this script: SC2086:info:1:17: Double quote to prevent globbing and word splitting

(shellcheck)


59-59: shellcheck reported issue in this script: SC2086:info:2:13: Double quote to prevent globbing and word splitting

(shellcheck)

🪛 yamllint (1.35.1)
.github/workflows/test.yml

[error] 1-1: wrong new line character: expected \n

(new-lines)

.github/workflows/deploy-stage.yml

[error] 1-1: wrong new line character: expected \n

(new-lines)

.devcontainer/docker-compose.yml

[error] 1-1: wrong new line character: expected \n

(new-lines)

.github/workflows/deploy-dev.yml

[error] 1-1: wrong new line character: expected \n

(new-lines)

.github/workflows/deploy-prod.yml

[error] 1-1: wrong new line character: expected \n

(new-lines)

🪛 Gitleaks (8.21.2)
.github/workflows/test.yml

32-32: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

.devcontainer/docker-compose.yml

38-38: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


118-118: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

API Collections/Auth/Web/Auth (magic link).bru

8-8: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)


14-14: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)

README.md

192-192: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 LanguageTool
src/services/item/plugins/app/docs/api.md

[style] ~17-~17: Try using a synonym here to strengthen your writing.
Context: ... strings

Within Graasp, the apps are given some information by query string:

  • ...

(GIVE_PROVIDE)


[uncategorized] ~29-~29: Loose punctuation mark.
Context: ... have the following structure:

  • id: the app action id
  • account: the mem...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~31-~31: Loose punctuation mark.
Context: ...nt authenticated member id).

  • member: use account
  • item: the item corre...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~56-~56: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...p-data">

App Data

App data are all data the app might save. They have ...

(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)


[uncategorized] ~58-~58: Loose punctuation mark.
Context: ... have the following structure:

  • id: the app data id
  • account: the membe...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~67-~67: Loose punctuation mark.
Context: ...them but cannot modify them.

  • item: the app data can be managed by the crea...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~68-~68: Loose punctuation mark.
Context: ...m but cannot modify them.

  • createdAt: creation timestamp of the app data
  • `...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~77-~77: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...an only be seen (get) by their creator, member, all admin. App data with visibility ...

(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)


[uncategorized] ~77-~77: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...be seen by all users, but only creator, member, and admin can patch/delete them.

Co...

(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)


[uncategorized] ~148-~148: A comma might be missing here.
Context: ...ion which might be interesting for your app such as:

  • members: a list of all ...

(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)


[uncategorized] ~150-~150: Loose punctuation mark.
Context: ...ing for your app such as:

  • members: a list of all the members having access...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~171-~171: Loose punctuation mark.
Context: ... have the following structure:

  • id: the app setting id
  • name: the app s...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~197-~197: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...{ data: { ... } }

  • permission: only member with the admin permission can patch an ...

(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)


[uncategorized] ~206-~206: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...e: deleted app data

  • permission: only member with the admin permission can delete an...

(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)


[uncategorized] ~245-~245: Loose punctuation mark.
Context: ...lorer, standalone`, ...)

  • itemId: item id which corresponds to your app r...

(UNLIKELY_OPENING_PUNCTUATION)

README.md

[typographical] ~11-~11: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...ent applications/frontends ?
Head over to: [Builder](https://github.com/graasp/gra...

(RP_COLON)


[style] ~14-~14: Consider a shorter alternative to avoid wordiness.
Context: ...p/graasp-account)

Requirements

In order to run the Graasp backend, it requires:

...

(IN_ORDER_TO_PREMIUM)


[uncategorized] ~20-~20: A comma may be missing after the conjunctive/linking adverb ‘However’.
Context: ...possible to install everything locally. However it is strongly recommanded to use the D...

(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)


[misspelling] ~26-~26: This word is normally spelled as one.
Context: ...environment (works on all shells and is super fast), requires you to use a [NerdFont](http...

(EN_COMPOUNDS_SUPER_FAST)


[style] ~27-~27: Consider shortening or rephrasing this to strengthen your wording.
Context: ...o.com) : IDE to manage the database and make changes to the source code.

  • [Remote-Contain...

(MAKE_CHANGES)


[grammar] ~29-~29: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ension for VS Code. It allows to easily setup the dev environnement.

  • [SQLTools...

(NOUN_VERB_CONFUSION)


[grammar] ~42-~42: Did you mean “using”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ... environment using Docker, as it allows to use a preconfigured developement environnem...

(ALLOW_TO)


[uncategorized] ~48-~48: Loose punctuation mark.
Context: ...eate 10 containers :

  • graasp-core : Node.js backend of Graasp
  • db : Pos...

(UNLIKELY_OPENING_PUNCTUATION)


[grammar] ~53-~53: Did you mean to write ‘used to’?
Context: ...raasp-localstack` : Localstack instance use to fake S3 storage locally

  • `localfil...

(USE_TO_NP)


[formatting] ~61-~61: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...un this command in the docker's terminal, because some packages are built depending on th...

(COMMA_BEFORE_BECAUSE)


[uncategorized] ~61-~61: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...uilt depending on the operating system (eg. bcrypt).

Info
If the proc...

(E_G)


[grammar] ~75-~75: Did you mean “Google Analytics”?
Context: ...raasp: the db for the backend

  • Umami: google analytics replacement
  • Etherpad: realtime docum...

(GOOGLE_PRODUCTS)


[misspelling] ~258-~258: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ttps://mailcatcher.me/), which emulates a SMTP server for sending e-mails. When u...

(EN_A_VS_AN)


[misspelling] ~260-~260: The word ‘informations’ is a legal term. In standard English, the word ‘information’ is a non-count noun.
Context: ...issues section](#known-issues) for more informations). The server is available at `http://lo...

(INFORMATIONS)


[misspelling] ~274-~274: Did you mean “lose” (= miss, waste, suffer the loss etc.)?
Context: ...database between test runs you will not loose your development data.

It's also imp...

(LOOSE_LOSE)


[grammar] ~286-~286: The verb ‘generate’ does not usually follow articles like ‘the’. Check that ‘generate’ is spelled correctly; using ‘generate’ as a noun may be non-standard.
Context: ...s files so typeorm can run them.

Run the generate and run command to create and apply the...

(A_INFINITIVE)


[typographical] ~318-~318: After an expression of time at the start of a sentence, a comma is usually used.
Context: ...tored on the fake s3 are not.

In the future we might investigate different solution...

(IN_THE_PAST_COMMA)


[style] ~318-~318: Consider an alternative to strengthen your wording.
Context: ....

In the future we might investigate different solutions to mocking s3 storage, or improve the l...

(DIFFERENT_ALTERNATIVE)


[grammar] ~328-~328: The word ‘lint’ is a noun or an adjective. A verb is missing or misspelled, or maybe a comma is missing.
Context: ...``sh
yarn openapi:generate


You can then lint the specifications with

```sh
yarn ...

(PRP_MD_NN)

</details>
<details>
<summary>docs/architecture.md</summary>

[uncategorized] ~9-~9: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...gins. Each plugin represents a feature (eg. member, item like, favorite, etc). Each...

(E_G)

---

[style] ~9-~9: In American English, abbreviations like “etc.” require a period.
Context: ...ature (eg. member, item like, favorite, etc). Each plugin is structured with one or...

(ETC_PERIOD)

---

[style] ~16-~16: The verb “get” can be informal. Consider replacing it with a form of “to be”.
Context: ...nction can run pre- and post-hooks that get registered outside of the service. (to be removed?...

(GET_USED_ARE_USED)

---

[style] ~16-~16: This phrase is redundant. Consider using “outside”.
Context: ...pre- and post-hooks that get registered outside of the service. (to be removed?)
  - If p...

(OUTSIDE_OF)

</details>
<details>
<summary>src/services/item/plugins/invitation/specifications.md</summary>

[style] ~34-~34: Consider an alternative for the often overused word ‘important’.
Context: ...TES**:

- Order of the columns is not important.
- Spaces around the delimiter are sup...

(NOT_IMPORTANT)

---

[uncategorized] ~39-~39: Possible missing comma found.
Context: ...ted

#### Valid CSV files

A single user specified only by email, will receive `...

(AI_HYDRA_LEO_MISSING_COMMA)

---

[uncategorized] ~98-~98: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ready holds an invitation for the member but they need to follow [membership inherit...

(COMMA_COMPOUND_SENTENCE)

</details>
<details>
<summary>src/services/item/plugins/publication/README.md</summary>

[uncategorized] ~67-~67: Loose punctuation mark.
Context: ...tionStatus`.
    - `401 Not Authorized`: If the user is not authenticated.
    ...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~68-~68: Loose punctuation mark.
Context: ...ot authenticated.
    - `403 Forbidden`: If the user is not administrator of the...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~69-~69: Loose punctuation mark.
Context: ...ator of the item.
    - `404 Not Found`: If the item is not found.

## Validat...

(UNLIKELY_OPENING_PUNCTUATION)

---

[grammar] ~73-~73: Did you mean “validating”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...e

The `ItemValidationService` allows to validate an item or to retrieve an item's valida...

(ALLOW_TO)

---

[uncategorized] ~84-~84: Loose punctuation mark.
Context: ...uccessfully.
    - `401 Not Authorized`: If the user is not authenticated.
    ...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~85-~85: Loose punctuation mark.
Context: ...ot authenticated.
    - `403 Forbidden`: If the user is not administrator of the...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~86-~86: Loose punctuation mark.
Context: ...ator of the item.
    - `404 Not Found`: If the item is not found.

## Publish...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~100-~100: Loose punctuation mark.
Context: ...ully published.
    - `400 Bad Request`: If the item cannot be published (missin...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~101-~101: Loose punctuation mark.
Context: ... public...).
    - `401 Not Authorized`: If the user is not authenticated.
    ...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~102-~102: Loose punctuation mark.
Context: ...ot authenticated.
    - `403 Forbidden`: If the user is not administrator of the...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~103-~103: Loose punctuation mark.
Context: ...ator of the item.
    - `404 Not Found`: If the item is not found.
- **Endpoint...

(UNLIKELY_OPENING_PUNCTUATION)

---

[grammar] ~109-~109: Did you mean to use the possessive pronoun “its”?
Context: ...tem is updated after being unpublished, it's state will be `Unpublished`.
  - **Res...

(IT_S_ITS)

---

[uncategorized] ~112-~112: Loose punctuation mark.
Context: ...unpublished.
    - `401 Not Authorized`: If the user is not authenticated.
    ...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~113-~113: Loose punctuation mark.
Context: ...ot authenticated.
    - `403 Forbidden`: If the user is not administrator of the...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~114-~114: Loose punctuation mark.
Context: ...ator of the item.
    - `404 Not Found`: If the item is not found or not publish...

(UNLIKELY_OPENING_PUNCTUATION)

</details>
<details>
<summary>src/services/item/plugins/recycled/README.md</summary>

[uncategorized] ~15-~15: The determiner “the” seems more likely in this context.
Context: ...f A, recycled

The user will see B in its trash.

### Admin Permission on child...

(AI_EN_LECTOR_REPLACEMENT_DETERMINER)

---

[style] ~25-~25: ‘taken into account’ might be wordy. Consider a shorter alternative.
Context: ...t anymore nor remove it. This should be taken into account when computing the storage of the user....

(EN_WORDINESS_PREMIUM_TAKEN_INTO_ACCOUNT)

</details>
<details>
<summary>src/services/itemMembership/specifications.md</summary>

[style] ~5-~5: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...itance rules

- A children membership can not be more restrictive (lower permission) ...

(CAN_NOT_PREMIUM)

</details>
<details>
<summary>src/services/websockets/API.md</summary>

[grammar] ~7-~7: It’s possible that the countable noun ‘placeholder’ after the verb ‘are’ is an error. You may have meant to use the plural form, or an article may be missing.
Context: ...ween angle brackets (`"<example>"`) are placeholder for dynamic values unless specified oth...

(BE_VB_OR_NN)

---

[uncategorized] ~22-~22: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...`action` field to perform subscriptions changes requests. Clients may send one of the f...

(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)

---

[grammar] ~24-~24: There seems to be a noun/verb agreement error. Did you mean “subscribes” or “subscribed”?
Context: ... of the following messages:

- Client subscribe: subscribe the WS client to the specifi...

(SINGULAR_NOUN_VERB_AGREEMENT)

---

[typographical] ~101-~101: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...` is `error`, then the `error` field is populated, otherwise it is `undefined`. The error has a name...

(THUS_SENTENCE)

---

[uncategorized] ~106-~106: Loose punctuation mark.
Context: ...messages include:

  - `ACCESS_DENIED`: the client is not allowed to access the...

(UNLIKELY_OPENING_PUNCTUATION)

</details>
<details>
<summary>src/services/websockets/USAGE.md</summary>

[uncategorized] ~15-~15: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...terface when updates of children items (e.g additions and deletions from other user...

(E_G)

---

[uncategorized] ~61-~61: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ... provides the interface for your feature but there is no React hook to consume the d...

(COMMA_COMPOUND_SENTENCE_2)

</details>

</details>
<details>
<summary>🪛 Markdownlint (0.37.0)</summary>

<details>
<summary>src/services/item/plugins/app/docs/api.md</summary>

7-7: Expected: h2; Actual: h3
Heading levels should only increment by one level at a time

(MD001, heading-increment)

</details>
<details>
<summary>README.md</summary>

245-245: null
Bare URL used

(MD034, no-bare-urls)

---

3-3: null
No empty links

(MD042, no-empty-links)

---

6-6: null
Images should have alternate text (alt text)

(MD045, no-alt-text)

</details>

</details>
<details>
<summary>🪛 Shellcheck (0.10.0)</summary>

<details>
<summary>.devcontainer/localstack/init.sh</summary>

[error] 1-1: Literal carriage return. Run script through tr -d '\r' .

(SC1017)

---

[error] 2-2: Literal carriage return. Run script through tr -d '\r' .

(SC1017)

---

[error] 3-3: Literal carriage return. Run script through tr -d '\r' .

(SC1017)

---

[error] 35-35: Literal carriage return. Run script through tr -d '\r' .

(SC1017)

---

[error] 36-36: Literal carriage return. Run script through tr -d '\r' .

(SC1017)

---

[error] 37-37: Literal carriage return. Run script through tr -d '\r' .

(SC1017)

---

[error] 38-38: Literal carriage return. Run script through tr -d '\r' .

(SC1017)

---

[error] 39-39: Literal carriage return. Run script through tr -d '\r' .

(SC1017)

</details>

</details>

</details>

<details>
<summary>🔇 Additional comments (10)</summary>

<details>
<summary>src/services/item/plugins/app/constants.ts (1)</summary>

`1-1`: **Verify JWT expiration time increase**

The JWT expiration time has been increased from 30 to 300 seconds. While 5 minutes is a reasonable duration and balances security with user experience, please ensure this aligns with your security requirements.



Consider implementing token refresh mechanisms if longer sessions are needed rather than extending the token lifetime further.

</details>
<details>
<summary>src/fastify.ts (1)</summary>

`14-14`: **Verify file size limits across the application**

The 25MB body limit is reasonable for file uploads, but ensure this aligns with:
- Individual file size limits in your upload handlers
- AWS configuration (e.g., ALB/API Gateway limits)
- Frontend upload size restrictions

</details>
<details>
<summary>.aws/graasp-dev.json (2)</summary>

`18-18`: **Verify container naming consistency**

The container names and log groups have been updated to reflect production environment:
- Main container: "lnco-ai-production"
- Iframely container: "lnco-ai-production-iframely"
- Log groups: "/ecs/lnco-ai-production" and "/ecs/lnco-ai-iframely-production"





Also applies to: 44-44, 53-53, 103-103

<details>
<summary>✅ Verification successful</summary>

**Container naming is consistent and correctly updated**

The container names and log groups in `.aws/graasp-dev.json` have been correctly updated to reflect the production environment:
- Main container name: "lnco-ai-production"
- Iframely container name: "lnco-ai-production-iframely"
- Log groups: "/ecs/lnco-ai-production" and "/ecs/lnco-ai-iframely-production"

All references are consistent within the configuration file and follow the same naming pattern.



</details>

<details>
<summary>🏁 Scripts executed</summary>

The following scripts were executed for the analysis:

Script:

```shell
#!/bin/bash
# Check for any remaining references to old container names
rg -g '!.git' 'graasp[-_](?:dev|prod|stage)'

Length of output: 46


Script:

#!/bin/bash
# Search for container and log group names in AWS config files
rg -g '*.json' 'awslogs-group|containerName'

# Search for any ECS or container related configuration
fd -e json -e yaml -e yml . .aws/ --exec cat {}

Length of output: 8266


5-6: Verify resource allocations for production workload

The current allocations (1 vCPU, 3GB memory) might be insufficient for production workload. Consider monitoring resource utilization to ensure these limits don't become a bottleneck.

Consider implementing auto-scaling policies based on CPU and memory metrics to handle varying loads.

.github/workflows/deploy-stage.yml (1)

147-166: Verify environment variables for Redis caching.

Given the PR's objective to fix caching issues, ensure these Redis-related environment variables are properly configured in the staging environment:

  • REDIS_HOST
  • REDIS_PORT
  • REDIS_PASSWORD
  • REDIS_USERNAME
✅ Verification successful

Redis environment variables are properly configured in the staging workflow

The Redis-related environment variables are correctly set up in the staging environment:

  • All required variables (REDIS_HOST, REDIS_PORT, REDIS_PASSWORD, REDIS_USERNAME) are present in .github/workflows/deploy-stage.yml
  • These variables are properly sourced from GitHub secrets
  • The configuration is consistent with other deployment environments (dev and prod)
  • The codebase actively uses Redis through the ioredis package, particularly for websockets and caching services
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if Redis configuration exists in environment files
echo "Checking Redis configuration in environment files..."
rg -l 'REDIS_(HOST|PORT|PASSWORD|USERNAME)' .env.*

Length of output: 216


Script:

#!/bin/bash
# Let's check for Redis configuration across the codebase
echo "Checking Redis configuration in the codebase..."
rg -A 2 'REDIS_(HOST|PORT|PASSWORD|USERNAME)' --type yaml

# Also check for Redis-related code
echo -e "\nChecking Redis-related code..."
rg -l 'redis|Redis' --type typescript --type javascript

# Check for Redis in package dependencies
echo -e "\nChecking Redis in package.json..."
fd package.json --exec cat {} | jq -r '.dependencies,.devDependencies' 2>/dev/null | grep -i redis || true

Length of output: 528


Script:

#!/bin/bash
# Let's check for Redis configuration in deployment files
echo "Checking Redis configuration in deployment files..."
rg -A 2 'REDIS_(HOST|PORT|PASSWORD|USERNAME)' .github/workflows/

# Check for Redis implementation files
echo -e "\nChecking Redis implementation files..."
fd -e js -e ts | xargs rg -l 'new Redis|ioredis'

# Check for Redis configuration patterns
echo -e "\nChecking Redis configuration patterns..."
rg -A 5 'redis.*config|Redis.*config' -g '!node_modules'

Length of output: 6558

src/utils/config.ts (3)

Line range hint 293-297: Verify file storage configuration for new AWS account.

Since this PR involves redeployment to a new AWS account, ensure the S3 configuration is properly updated:

  1. Bucket permissions
  2. IAM roles and policies
  3. CORS configuration

Line range hint 261-264: Review Redis caching configuration.

Given the PR's objective to fix caching issues, verify the Redis configuration:

  1. Connection pool settings
  2. Cache invalidation strategy
  3. Error handling for Redis connection failures
✅ Verification successful

Let me try a different approach to gather Redis-related configuration and error handling information.

This script will help us:

  1. Locate Redis-related files
  2. Find Redis connection and configuration patterns
  3. Identify cache-related code that might be relevant to the PR's objective

Based on the shell script results, I can now provide a final response regarding the Redis caching configuration review:

Redis configuration and error handling are properly implemented

The codebase shows a well-structured Redis implementation with:

  1. Proper configuration in src/utils/config.ts:

    • Host, port, username, and password settings
    • Default port fallback (6379)
  2. Robust error handling in src/services/websockets/multi-instance.ts:

    redis.on('error', (err) => {
      log?.error(`graasp-plugin-websockets: MultiInstanceChannelsBroker failed to connect to Redis instance, reason:\n\t${err}`);
    });
  3. Clean caching service implementation in src/services/caching/service.ts:

    • Proper key namespacing
    • Expiration handling
    • Get/set/delete operations with error boundaries

The Redis configuration and error handling appear to be complete and following best practices. No issues were found in the caching implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Look for Redis-related error handling
echo "Checking Redis error handling..."
ast-grep --pattern 'try {
  $$$
  redis.$_($$$)
  $$$
} catch (error) {
  $$$
}'

Length of output: 157


Script:

#!/bin/bash
# Search for Redis-related configurations and error handling patterns
echo "Searching for Redis configurations and connections..."
rg -l "redis" --type ts

echo -e "\nSearching for Redis-related code..."
rg "redis|Redis" -i --type ts -A 3

echo -e "\nSearching for cache-related configurations..."
rg "cache" --type ts -A 3

Length of output: 37998


145-145: Verify the impact of auth URL change.

The authentication URL has been changed from 'https://mobile.graasp.org' to 'https://auth.lnco.ai'. Ensure this change is coordinated with:

  1. Mobile client applications
  2. Authentication service deployment
  3. DNS configuration
✅ Verification successful

URL change is properly integrated and used consistently

The change from 'https://mobile.graasp.org' to 'https://auth.lnco.ai' is properly integrated in the codebase:

  • The URL is used consistently in the authentication flow for mobile clients
  • It's correctly referenced in email templates
  • The code properly handles the URL through environment variables with a fallback
  • The authentication service uses this URL appropriately for challenge-based authentication flows

The implementation shows that:

  1. The URL is used for authentication redirects when challenges are present
  2. Email templates are updated to use the new domain
  3. The change is properly coordinated with the mobile authentication flow
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for other occurrences of the old URL
echo "Checking for other occurrences of mobile.graasp.org..."
rg -l 'mobile\.graasp\.org'

# Check for related configuration in mobile clients
echo "Checking for auth URL configuration in mobile-related files..."
fd -t f -e json -e ts -e js -e yaml -e yml | xargs rg 'auth.*\.lnco\.ai'

Length of output: 839


Script:

#!/bin/bash
# Let's check for any references to the old URL pattern in configuration files and environment templates
echo "Checking for environment files and templates..."
fd -e env -e env.template -e env.example

# Check for any mobile-related configuration files
echo "Checking for mobile configuration files..."
fd -t f mobile

# Check for authentication-related configuration
echo "Checking authentication configuration..."
rg -l -i "auth.*url|authentication.*url"

# Check for mobile-related code that might use authentication
echo "Checking mobile-related authentication code..."
ast-grep --pattern 'const $_ = $$$auth$$$'

Length of output: 1136


Script:

#!/bin/bash
# Let's examine the mobile authentication related files to see the URL usage
echo "Checking mobile authentication plugin..."
cat src/services/auth/plugins/mobile/index.ts

echo "\nChecking mobile authentication schemas..."
cat src/services/auth/plugins/mobile/schemas.ts

echo "\nChecking auth service configuration..."
cat src/services/auth/service.ts

# Look for environment variable usage
echo "\nChecking for MOBILE_AUTH_URL environment variable references..."
rg "MOBILE_AUTH_URL" -A 2 -B 2

Length of output: 13944

src/services/item/plugins/invitation/specifications.md (1)

1-98: Verify alignment with PR objectives

The PR description mentions fixing caching issues affecting file upload functionality, but the documentation changes reviewed don't reflect any caching-related modifications. Please ensure that:

  1. The caching changes are included in other files not shown here
  2. The documentation is updated to reflect any architectural changes related to caching
✅ Verification successful

Documentation and implementation are correctly aligned

The code shows evidence of caching functionality in the file service that's currently commented out, suggesting recent changes to address caching issues. The documentation in specifications.md correctly describes the CSV upload functionality without needing to expose implementation details about caching, as these are internal implementation concerns.

Key findings in src/services/file/service.ts:

  • Commented out caching operations in upload/delete methods
  • CachingService integration exists but appears to be under modification
  • File upload functionality is present and matches the documented API
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for cache-related changes in the codebase
rg -l "cache|caching" --type ts

Length of output: 264


Script:

#!/bin/bash
# Check for recent changes in cache-related files
rg -B2 -A2 "cache|caching" src/services/file/service.ts

# Look for file upload related code
rg -B2 -A2 "upload" src/services/file/service.ts

# Check for multipart form handling
rg -B2 -A2 "multipart|FormData" src/services/file/service.ts

Length of output: 1476

🧰 Tools
🪛 LanguageTool

[style] ~34-~34: Consider an alternative for the often overused word ‘important’.
Context: ...TES**:

  • Order of the columns is not important.
  • Spaces around the delimiter are sup...

(NOT_IMPORTANT)


[uncategorized] ~39-~39: Possible missing comma found.
Context: ...ted

Valid CSV files

A single user specified only by email, will receive `...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~98-~98: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...ready holds an invitation for the member but they need to follow [membership inherit...

(COMMA_COMPOUND_SENTENCE)

.github/workflows/deploy-prod.yml (1)

38-40: ⚠️ Potential issue

Remove TODO comments and properly configure AWS credentials.

The workflow contains TODO comments about adding AWS credentials to environments. These should be addressed before deploying to production.

Comment on lines 15 to 17
"INVITATION_TEXT": "{{creatorName}} shared {{itemName}} with you on the LNCO.ai platform. Click on the following button to complete your registration.",
"INVITATION_TITLE": "The item {{itemName}} was shared with you on LNCO.ai",
"PUBLISH_ITEM_TEXT": "Your item {{itemName}} has been published to LNCO.ai Library. Click on the following link to view it.",
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent branding and URLs found across email templates

  • The English templates have been updated to use LNCO.ai branding and auth.lnco.ai URLs
  • Other language templates (it, fr, es, ar, de) still reference graasp.org
  • The configuration shows MOBILE_AUTH_URL is set to auth.lnco.ai, but several other configurations still use graasp.org domains
🔗 Analysis chain

Verify accessibility of new auth.lnco.ai domain

The URLs have been updated to use auth.lnco.ai domain. Please ensure:

  1. The domain is properly configured and accessible
  2. SSL certificates are valid
  3. DNS records are properly set up

Also applies to: 23-27

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any remaining references to old domains
rg -g '!.git' 'graasp\.org'

# Check for consistency in new domain usage
rg -g '!.git' 'lnco\.ai'

Length of output: 5509

Comment on lines 15 to 17
"INVITATION_TEXT": "{{creatorName}} a partagé {{itemName}} avec vous sur la plateforme Graasp. Cliquez sur le bouton suivant pour compléter votre inscription.",
"INVITATION_TITLE": "Cet élément a été partagé avec vous sur Graasp",
"PUBLISH_ITEM_TEXT": "Votre élément {{itemName}} a été publié sur Graasp Library. Cliquez sur le lien suivant pour le voir.",
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Update French translations to match LNCO.ai rebranding

The French translations still reference "Graasp" and "graasp.org" while the English version has been updated to "LNCO.ai" and "auth.lnco.ai". Please update the following strings:

-  "INVITATION_TEXT": "{{creatorName}} a partagé {{itemName}} avec vous sur la plateforme Graasp...",
+  "INVITATION_TEXT": "{{creatorName}} a partagé {{itemName}} avec vous sur la plateforme LNCO.ai...",
-  "INVITATION_TITLE": "Cet élément a été partagé avec vous sur Graasp",
+  "INVITATION_TITLE": "Cet élément a été partagé avec vous sur LNCO.ai",
-  "SHARE_ITEM_TEXT": "L'élément {{itemName}} a été partagé avec vous sur Graasp...",
+  "SHARE_ITEM_TEXT": "L'élément {{itemName}} a été partagé avec vous sur LNCO.ai...",
-  "FOOTER": "Cet email vous a été envoyé car vous avez un compte sur <a href=\"https://graasp.org\">Graasp</a>",
+  "FOOTER": "Cet email vous a été envoyé car vous avez un compte sur <a href=\"https://auth.lnco.ai\">LNCO.ai</a>",
-  "POWERED_BY": "Conçu par <a href=\"https://graasp.org\">Graasp</a>.",
+  "POWERED_BY": "Conçu par <a href=\"https://auth.lnco.ai\">LNCO.ai</a>."

Also applies to: 23-27

Comment on lines 79 to 90
test-db:
container_name: graasp-postgres-test
image: postgres:15
restart: on-failure
ports:
- 5434:5432
environment:
POSTGRES_DB: docker-test
POSTGRES_USER: docker-test
POSTGRES_PASSWORD: docker-test
# no volumes since we don't care about perssissting the data

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add health checks to test-db service.

The test-db service is missing health checks that are present in the main db service. This could lead to race conditions in tests if the database isn't ready when tests begin.

Add the following health check configuration:

  test-db:
    container_name: graasp-postgres-test
    image: postgres:15
    restart: on-failure
    ports:
      - 5434:5432
    environment:
      POSTGRES_DB: docker-test
      POSTGRES_USER: docker-test
      POSTGRES_PASSWORD: docker-test
+   healthcheck:
+     test: ["CMD-SHELL", "pg_isready -U docker-test -d docker-test"]
+     interval: 5s
+     timeout: 5s
+     retries: 5
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test-db:
container_name: graasp-postgres-test
image: postgres:15
restart: on-failure
ports:
- 5434:5432
environment:
POSTGRES_DB: docker-test
POSTGRES_USER: docker-test
POSTGRES_PASSWORD: docker-test
# no volumes since we don't care about perssissting the data
test-db:
container_name: graasp-postgres-test
image: postgres:15
restart: on-failure
ports:
- 5434:5432
environment:
POSTGRES_DB: docker-test
POSTGRES_USER: docker-test
POSTGRES_PASSWORD: docker-test
healthcheck:
test: ["CMD-SHELL", "pg_isready -U docker-test -d docker-test"]
interval: 5s
timeout: 5s
retries: 5
# no volumes since we don't care about perssissting the data

}

get {
url: {{host}}/auth?t=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIyMGJmZGZiNC1iZjZjLTQ3ZDUtOGRlMi01MjRhNTliNDQ1MTkiLCJpYXQiOjE3MDI0NTcwNjcsImV4cCI6MTcwMjQ1ODg2N30.Gb-6vKFY3feVL7_DBmVHQuptFAxZqrmwpNnZrTxoFw8
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Security: Remove hardcoded JWT tokens from version control

The API collection contains hardcoded JWT tokens in both the URL and query parameters. Even though this is a testing/documentation file, storing tokens in version control poses a security risk as they may contain sensitive information.

Recommendations:

  1. Replace tokens with environment variables: {{auth_token}}
  2. Document how to generate test tokens in the collection's documentation
  3. Add .env files to .gitignore to prevent accidental token commits
-  url: {{host}}/auth?t=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...
+  url: {{host}}/auth?t={{auth_token}}

-  t: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...
+  t: {{auth_token}}

Also applies to: 14-14

🧰 Tools
🪛 Gitleaks (8.21.2)

8-8: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.

(jwt)

Comment on lines +1 to +39
#!/bin/bash
bucketName=graasp
corsConfig='{
"CORSRules": [
{
"AllowedHeaders": [
"*"
],
"AllowedMethods": [
"HEAD",
"PUT",
"GET",
"DELETE"
],
"AllowedOrigins": [
"null"
],
"ExposeHeaders": []
},
{
"AllowedHeaders": [
"*"
],
"AllowedMethods": [
"HEAD",
"GET"
],
"AllowedOrigins": [
"*"
],
"ExposeHeaders": []
}
]
}'
awslocal s3 mb s3://$bucketName
awslocal s3api put-bucket-cors --bucket $bucketName --cors-configuration "${corsConfig}"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix CRLF line endings

The script contains Windows-style (CRLF) line endings which could cause execution issues in Linux environments.

Run this command to fix line endings:

tr -d '\r' < init.sh > init.sh.tmp && mv init.sh.tmp init.sh
🧰 Tools
🪛 Shellcheck (0.10.0)

[error] 1-1: Literal carriage return. Run script through tr -d '\r' .

(SC1017)


[error] 2-2: Literal carriage return. Run script through tr -d '\r' .

(SC1017)


[error] 3-3: Literal carriage return. Run script through tr -d '\r' .

(SC1017)


[error] 35-35: Literal carriage return. Run script through tr -d '\r' .

(SC1017)


[error] 36-36: Literal carriage return. Run script through tr -d '\r' .

(SC1017)


[error] 37-37: Literal carriage return. Run script through tr -d '\r' .

(SC1017)


[error] 38-38: Literal carriage return. Run script through tr -d '\r' .

(SC1017)


[error] 39-39: Literal carriage return. Run script through tr -d '\r' .

(SC1017)

id: image-url
env:
TARGET_IMAGE_URL: ${{ steps.login-ecr-build.outputs.registry }}/${{ vars.ECR_REPOSITORY }}:${{ github.ref_name || github.ref || github.sha }}
run: echo "image_url=${TARGET_IMAGE_URL}" >> $GITHUB_OUTPUT
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add proper quoting to shell commands

The shell commands should use proper quoting to prevent globbing and word splitting:

Apply these changes:

- echo "image_url=${TARGET_IMAGE_URL}" >> $GITHUB_OUTPUT
+ echo "image_url=${TARGET_IMAGE_URL}" >> "$GITHUB_OUTPUT"

- docker build -t $TARGET_IMAGE_URL -f docker/Dockerfile --build-arg APP_VERSION=${{ github.sha }} .
- docker push $TARGET_IMAGE_URL
+ docker build -t "$TARGET_IMAGE_URL" -f docker/Dockerfile --build-arg APP_VERSION=${{ github.sha }} .
+ docker push "$TARGET_IMAGE_URL"

Also applies to: 58-59

🧰 Tools
🪛 actionlint (1.7.4)

50-50: shellcheck reported issue in this script: SC2086:info:1:41: Double quote to prevent globbing and word splitting

(shellcheck)

Comment on lines +129 to +166
JOB_SCHEDULING=${{ vars.JOB_SCHEDULING || false }}
JWT_SECRET=${{ secrets.JWT_SECRET }}
PASSWORD_RESET_JWT_SECRET=${{ secrets.PASSWORD_RESET_JWT_SECRET }}
EMAIL_CHANGE_JWT_SECRET=${{ secrets.EMAIL_CHANGE_JWT_SECRET }}
LIBRARY_CLIENT_HOST=${{ vars.LIBRARY_CLIENT_HOST }}
LOG_LEVEL=${{ vars.LOG_LEVEL }}
MAILER_CONFIG_FROM_EMAIL=${{ secrets.MAILER_CONFIG_FROM_EMAIL }}
MAILER_CONFIG_PASSWORD=${{ secrets.MAILER_CONFIG_PASSWORD }}
MAILER_CONFIG_SMTP_HOST=${{ secrets.MAILER_CONFIG_SMTP_HOST }}
MAILER_CONFIG_USERNAME=${{ secrets.MAILER_CONFIG_USERNAME }}
MEILISEARCH_MASTER_KEY=${{ secrets.MEILISEARCH_MASTER_KEY }}
MEILISEARCH_REBUILD_SECRET=${{ secrets.MEILISEARCH_REBUILD_SECRET }}
MEILISEARCH_URL=${{ secrets.MEILISEARCH_URL }}
NODE_ENV=${{ vars.NODE_ENV }}
PLAYER_CLIENT_HOST=${{ vars.PLAYER_CLIENT_HOST }}
PORT=${{ vars.PORT }}
PUBLIC_URL=${{ secrets.PUBLIC_URL }}
RECAPTCHA_SECRET_ACCESS_KEY=${{ secrets.RECAPTCHA_SECRET_ACCESS_KEY }}
REDIS_HOST=${{ secrets.REDIS_HOST }}
REDIS_PASSWORD=${{ secrets.REDIS_PASSWORD }}
REDIS_PORT=${{ secrets.REDIS_PORT }}
REDIS_USERNAME=${{ secrets.REDIS_USERNAME }}
REFRESH_TOKEN_EXPIRATION_IN_MINUTES=${{ vars.REFRESH_TOKEN_EXPIRATION_IN_MINUTES }}
REFRESH_TOKEN_JWT_SECRET=${{ secrets.REFRESH_TOKEN_JWT_SECRET }}
S3_FILE_ITEM_ACCESS_KEY_ID=${{ secrets.S3_FILE_ITEM_ACCESS_KEY_ID }}
S3_FILE_ITEM_BUCKET=${{ vars.S3_FILE_ITEM_BUCKET }}
S3_FILE_ITEM_PLUGIN=${{ vars.S3_FILE_ITEM_PLUGIN }}
S3_FILE_ITEM_REGION=${{ vars.S3_FILE_ITEM_REGION }}
S3_FILE_ITEM_SECRET_ACCESS_KEY=${{ secrets.S3_FILE_ITEM_SECRET_ACCESS_KEY }}
SECURE_SESSION_SECRET_KEY=${{ secrets.SECURE_SESSION_SECRET_KEY }}
SENTRY_DSN=${{ secrets.SENTRY_DSN }}
TOKEN_BASED_AUTH=${{ vars.TOKEN_BASED_AUTH }}
WEBSOCKETS_PLUGIN=${{ vars.WEBSOCKETS_PLUGIN }}
OPENAI_API_KEY=${{ secrets.OPENAI_API_KEY }}
OPENAI_ORG_ID=${{ secrets.OPENAI_ORG_ID }}
GEOLOCATION_API_HOST=${{ vars.GEOLOCATION_API_HOST }}
GEOLOCATION_API_KEY=${{ secrets.GEOLOCATION_API_KEY }}

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Review environment variables for sensitive data exposure.

The environment variables section contains multiple sensitive configurations. Consider:

  1. Moving sensitive values to GitHub Secrets
  2. Using environment-specific variable groups
  3. Implementing secret rotation for API keys and tokens

id: image-url
env:
TARGET_IMAGE_URL: ${{ steps.login-ecr-build.outputs.registry }}/${{ vars.ECR_REPOSITORY }}:${{ github.event.client_payload.tag }}
run: echo "image_url=${TARGET_IMAGE_URL}" >> $GITHUB_OUTPUT
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential shell script injection vulnerabilities.

The shell commands using environment variables should be properly quoted to prevent injection attacks.

Apply this diff:

-echo "image_url=${TARGET_IMAGE_URL}" >> $GITHUB_OUTPUT
+echo "image_url=${TARGET_IMAGE_URL@Q}" >> $GITHUB_OUTPUT

-docker build -t $TARGET_IMAGE_URL -f docker/Dockerfile --build-arg APP_VERSION=${{ github.event.client_payload.tag }} .
-docker push $TARGET_IMAGE_URL
+docker build -t "${TARGET_IMAGE_URL}" -f docker/Dockerfile --build-arg "APP_VERSION=${{ github.event.client_payload.tag }}" .
+docker push "${TARGET_IMAGE_URL}"

Also applies to: 60-61

🧰 Tools
🪛 actionlint (1.7.4)

52-52: shellcheck reported issue in this script: SC2086:info:1:41: Double quote to prevent globbing and word splitting

(shellcheck)

return (
<div>
{data.map((item) => (
<a>{item.name}</a>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix incorrect channel initialization in the example code.

The channel object is incorrectly using userId instead of bazId in the example.

Apply this diff:

-const channel: Channel = { name: userId, topic: 'baz' };
+const channel: Channel = { name: bazId, topic: 'baz' };

Also applies to: 151-151

# Optional, if the etherpad cookie domain is different from the domain of the public URL
# ETHERPAD_COOKIE_DOMAIN=localhost
# Api key is set by ./.devcontainer/etherpad/devApiKey.txt
# ETHERPAD_API_KEY=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove exposed API key from documentation.

The example contains a real API key pattern. Replace it with a placeholder.

Apply this diff:

-# ETHERPAD_API_KEY=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
+# ETHERPAD_API_KEY=<your-etherpad-api-key>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# ETHERPAD_API_KEY=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
# ETHERPAD_API_KEY=<your-etherpad-api-key>
🧰 Tools
🪛 Gitleaks (8.21.2)

192-192: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

Copy link

@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

♻️ Duplicate comments (1)
.devcontainer/docker-compose.yml (1)

79-90: 🛠️ Refactor suggestion

Add healthcheck to test-db service

While the test database configuration is appropriate for testing purposes, it's missing the healthcheck configuration that's present in the main database service. This could lead to race conditions in tests if the database isn't ready when tests begin.

Add the following configuration:

  test-db:
    container_name: graasp-postgres-test
    image: postgres:15
    restart: on-failure
    ports:
      - 5434:5432
    environment:
      POSTGRES_DB: docker-test
      POSTGRES_USER: docker-test
      POSTGRES_PASSWORD: docker-test
+   healthcheck:
+     test: ["CMD-SHELL", "pg_isready -U docker-test -d docker-test"]
+     interval: 5s
+     timeout: 5s
+     retries: 5
🧹 Nitpick comments (1)
.devcontainer/docker-compose.yml (1)

91-94: Document Redis service's role in caching strategy

The reintroduction of the Redis service appears to be related to the caching issues mentioned in the PR objectives. Consider:

  1. Documenting the intended use of Redis in the caching strategy
  2. Adding comments explaining why persistence is not needed
  3. Specifying if any additional Redis configuration will be needed in the future
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 74ce71e and e61d295.

📒 Files selected for processing (2)
  • .aws/graasp-prod.json (0 hunks)
  • .devcontainer/docker-compose.yml (7 hunks)
💤 Files with no reviewable changes (1)
  • .aws/graasp-prod.json
🔇 Additional comments (2)
.devcontainer/docker-compose.yml (2)

30-32: LGTM: Database credential standardization

The standardization of database credentials to use consistent values across services improves maintainability of the development environment.


54-55: Volume path modifications align with PR objectives

The addition of quotes to volume paths follows YAML best practices. These changes, particularly for file storage paths, may contribute to resolving the file upload caching issues mentioned in the PR objectives.

Consider documenting the rationale for these changes in relation to the caching issues for future reference.

Also applies to: 68-68, 110-110, 136-136, 138-138, 156-156

Copy link

@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 (8)
README.md (4)

3-3: Fix the empty GitHub Release badge link.

The GitHub Release badge has an empty link which makes it non-functional.

-[![GitHub Release](https://img.shields.io/github/release/graasp/graasp)]()
+[![GitHub Release](https://img.shields.io/github/release/graasp/graasp)](https://github.com/graasp/graasp/releases/latest)
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)

3-3: No empty links
null

(MD042, no-empty-links)


8-8: Fix typo in introduction.

There's a typo in the word "confgurations".

-This repository contains the source code and confgurations for the Graasp Backend. Visit the Graasp Platform at [graasp.org](https://graasp.org)
+This repository contains the source code and configurations for the Graasp Backend. Visit the Graasp Platform at [graasp.org](https://graasp.org)

268-268: Fix grammatical error in testing section.

The word "loose" is incorrect in this context. It should be "lose" (to be deprived of something).

-you will not loose your development data.
+you will not lose your development data.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~268-~268: Possible missing comma found.
Context: ...ey will clean the database between test runs you will not loose your development dat...

(AI_HYDRA_LEO_MISSING_COMMA)


[misspelling] ~268-~268: Did you mean “lose” (= miss, waste, suffer the loss etc.)?
Context: ...database between test runs you will not loose your development data.

It's also imp...

(LOOSE_LOSE)


254-254: Fix grammatical error in utilities section.

The word "informations" is incorrect. In standard English, "information" is a non-count noun.

-for more informations). The server
+for more information). The server
🧰 Tools
🪛 LanguageTool

[misspelling] ~254-~254: The word ‘informations’ is a legal term. In standard English, the word ‘information’ is a non-count noun.
Context: ...issues section](#known-issues) for more informations). The server is available at `http://lo...

(INFORMATIONS)

src/fastify.ts (1)

11-26: Consider implementing structured logging for better observability.

While removing problematic logging configurations might help with performance, maintaining proper observability in production is crucial. Consider implementing structured logging with:

  1. Different log levels for development and production
  2. JSON format for better parsing
  3. Correlation IDs for request tracing
  4. Sampling for high-volume endpoints

This would help balance performance with debugging capabilities.

.github/workflows/test.yml (1)

197-202: Consider using a dynamic Jest cache directory.

Instead of hardcoding .jest-cache, consider using yarn config get cacheFolder to determine the cache directory dynamically, similar to how the yarn cache is handled.

-          path: .jest-cache
+          path: ${{ steps.yarn-cache-dir-path.outputs.dir }}/jest
.github/workflows/deploy-stage.yml (2)

9-10: Consider adding garbage collection settings.

While increasing the max heap size is good, consider adding garbage collection settings to optimize memory usage during the build process.

-  NODE_OPTIONS: '--max_old_space_size=8192' # Allows to increase Node's max heap size
+  NODE_OPTIONS: '--max_old_space_size=8192 --expose-gc' # Allows to increase Node's max heap size and enable manual garbage collection

97-162: Consider grouping environment variables by category.

The environment variables could be organized into logical groups (e.g., database, authentication, storage, etc.) using comments for better readability and maintenance.

Example organization:

          environment-variables: |
+            # Database Configuration
            DATABASE_LOGS=${{ vars.DATABASE_LOGS }}
            DB_CONNECTION_POOL_SIZE=${{ vars.DB_CONNECTION_POOL_SIZE }}
            DB_HOST=${{ secrets.DB_HOST }}
            DB_NAME=${{ secrets.DB_NAME }}
            DB_PASSWORD=${{ secrets.DB_PASSWORD }}
            DB_READ_REPLICA_HOSTS=${{ secrets.DB_READ_REPLICA_HOSTS }}
            DB_USERNAME=${{ secrets.DB_USERNAME }}
+            # Authentication
            APPS_JWT_SECRET=${{ secrets.APPS_JWT_SECRET }}
            AUTH_TOKEN_EXPIRATION_IN_MINUTES=${{ secrets.AUTH_TOKEN_EXPIRATION_IN_MINUTES }}
            AUTH_TOKEN_JWT_SECRET=${{ secrets.AUTH_TOKEN_JWT_SECRET }}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b159e29 and 0d8da6c.

📒 Files selected for processing (14)
  • .aws/graasp-stage.json (1 hunks)
  • .devcontainer/docker-compose.yml (7 hunks)
  • .eslintrc (1 hunks)
  • .github/workflows/deploy-stage.yml (1 hunks)
  • .github/workflows/test.yml (1 hunks)
  • README.md (1 hunks)
  • src/fastify.ts (2 hunks)
  • src/langs/ar.json (1 hunks)
  • src/langs/de.json (1 hunks)
  • src/langs/es.json (1 hunks)
  • src/langs/it.json (1 hunks)
  • src/services/file/service.ts (2 hunks)
  • src/services/item/plugins/thumbnail/service.ts (0 hunks)
  • src/utils/config.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • src/services/item/plugins/thumbnail/service.ts
✅ Files skipped from review due to trivial changes (4)
  • src/langs/es.json
  • src/langs/ar.json
  • src/langs/de.json
  • src/langs/it.json
🚧 Files skipped from review as they are similar to previous changes (5)
  • .aws/graasp-stage.json
  • src/services/file/service.ts
  • .eslintrc
  • src/utils/config.ts
  • .devcontainer/docker-compose.yml
🧰 Additional context used
🧠 Learnings (1)
.github/workflows/test.yml (1)
Learnt from: ymijsters
PR: lnco-epfl/graasp#17
File: .github/workflows/test.yml:32-33
Timestamp: 2024-11-21T14:37:55.029Z
Learning: In the Graasp repository, the Etherpad API key is intentionally hardcoded in the `.github/workflows/test.yml` file to align with the main branch configuration.
🪛 YAMLlint (1.35.1)
.github/workflows/deploy-stage.yml

[error] 1-1: wrong new line character: expected \n

(new-lines)

.github/workflows/test.yml

[error] 1-1: wrong new line character: expected \n

(new-lines)

🪛 Gitleaks (8.21.2)
.github/workflows/test.yml

29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🪛 LanguageTool
README.md

[typographical] ~11-~11: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...ent applications/frontends ?
Head over to: [Builder](https://github.com/graasp/gra...

(RP_COLON)


[style] ~14-~14: Consider a shorter alternative to avoid wordiness.
Context: ...p/graasp-account)

Requirements

In order to run the Graasp backend, it requires:

...

(IN_ORDER_TO_PREMIUM)


[uncategorized] ~20-~20: A comma may be missing after the conjunctive/linking adverb ‘However’.
Context: ...possible to install everything locally. However it is strongly recommanded to use the D...

(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)


[misspelling] ~26-~26: This word is normally spelled as one.
Context: ...environment (works on all shells and is super fast), requires you to use a [NerdFont](http...

(EN_COMPOUNDS_SUPER_FAST)


[style] ~27-~27: Consider shortening or rephrasing this to strengthen your wording.
Context: ...o.com) : IDE to manage the database and make changes to the source code.

  • [Remote-Contain...

(MAKE_CHANGES)


[grammar] ~29-~29: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ension for VS Code. It allows to easily setup the dev environnement.

  • [SQLTools...

(NOUN_VERB_CONFUSION)


[grammar] ~42-~42: Did you mean “using”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ... environment using Docker, as it allows to use a preconfigured developement environnem...

(ALLOW_TO)


[uncategorized] ~48-~48: Loose punctuation mark.
Context: ...eate 10 containers :

  • graasp-core : Node.js backend of Graasp
  • db : Pos...

(UNLIKELY_OPENING_PUNCTUATION)


[grammar] ~53-~53: Did you mean to write ‘used to’?
Context: ...raasp-localstack` : Localstack instance use to fake S3 storage locally

  • `localfil...

(USE_TO_NP)


[formatting] ~61-~61: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...un this command in the docker's terminal, because some packages are built depending on th...

(COMMA_BEFORE_BECAUSE)


[uncategorized] ~61-~61: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...uilt depending on the operating system (eg. bcrypt).

Info
If the proc...

(E_G)


[uncategorized] ~67-~67: Possible missing comma found.
Context: ...(2 GB) to 8GB.

Once the packages are installed we need to bootstrap the database. Run ...

(AI_HYDRA_LEO_MISSING_COMMA)


[grammar] ~76-~76: Did you mean “Google Analytics”?
Context: ...raasp: the db for the backend

  • Umami: google analytics replacement
  • Etherpad: realtime docum...

(GOOGLE_PRODUCTS)


[uncategorized] ~81-~81: Possible missing comma found.
Context: ...ring tests

Local Installation

First a running and accessible instance of Po...

(AI_HYDRA_LEO_MISSING_COMMA)


[misspelling] ~252-~252: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ttps://mailcatcher.me/), which emulates a SMTP server for sending e-mails. When u...

(EN_A_VS_AN)


[misspelling] ~254-~254: The word ‘informations’ is a legal term. In standard English, the word ‘information’ is a non-count noun.
Context: ...issues section](#known-issues) for more informations). The server is available at `http://lo...

(INFORMATIONS)


[uncategorized] ~268-~268: Possible missing comma found.
Context: ...ey will clean the database between test runs you will not loose your development dat...

(AI_HYDRA_LEO_MISSING_COMMA)


[misspelling] ~268-~268: Did you mean “lose” (= miss, waste, suffer the loss etc.)?
Context: ...database between test runs you will not loose your development data.

It's also imp...

(LOOSE_LOSE)


[grammar] ~280-~280: The verb ‘generate’ does not usually follow articles like ‘the’. Check that ‘generate’ is spelled correctly; using ‘generate’ as a noun may be non-standard.
Context: ...s files so typeorm can run them.

Run the generate and run command to create and apply the...

(A_INFINITIVE)


[typographical] ~312-~312: After an expression of time at the start of a sentence, a comma is usually used.
Context: ...tored on the fake s3 are not.

In the future we might investigate different solution...

(IN_THE_PAST_COMMA)


[style] ~312-~312: Consider an alternative to strengthen your wording.
Context: ....

In the future we might investigate different solutions to mocking s3 storage, or improve the l...

(DIFFERENT_ALTERNATIVE)


[grammar] ~322-~322: The word ‘lint’ is a noun or an adjective. A verb is missing or misspelled, or maybe a comma is missing.
Context: ...``sh
yarn openapi:generate


You can then lint the specifications with

```sh
yarn ...

(PRP_MD_NN)

</details>

</details>
<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>

<details>
<summary>README.md</summary>

3-3: No empty links
null

(MD042, no-empty-links)

---

6-6: Images should have alternate text (alt text)
null

(MD045, no-alt-text)

---

240-240: Bare URL used
null

(MD034, no-bare-urls)

</details>

</details>

</details>

<details>
<summary>⏰ Context from checks skipped due to timeout of 90000ms (5)</summary>

* GitHub Check: test-node (4/4)
* GitHub Check: test-node (3/4)
* GitHub Check: test-node (2/4)
* GitHub Check: test-node (1/4)
* GitHub Check: Build & Push Image

</details>

<details>
<summary>🔇 Additional comments (8)</summary><blockquote>

<details>
<summary>README.md (2)</summary>

`191-191`: **Remove exposed API key from documentation.**

The example contains a real API key pattern. Replace it with a placeholder.

```diff
-# ETHERPAD_API_KEY=0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef
+# ETHERPAD_API_KEY=<your-etherpad-api-key>

1-327: Documentation is comprehensive and well-structured!

The README provides clear instructions for setup, configuration, and usage. The organization is logical, and the content is thorough.

🧰 Tools
🪛 LanguageTool

[typographical] ~11-~11: Do not use a colon (:) before a series that is introduced by a preposition (‘to’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...ent applications/frontends ?
Head over to: [Builder](https://github.com/graasp/gra...

(RP_COLON)


[style] ~14-~14: Consider a shorter alternative to avoid wordiness.
Context: ...p/graasp-account)

Requirements

In order to run the Graasp backend, it requires:

...

(IN_ORDER_TO_PREMIUM)


[uncategorized] ~20-~20: A comma may be missing after the conjunctive/linking adverb ‘However’.
Context: ...possible to install everything locally. However it is strongly recommanded to use the D...

(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)


[misspelling] ~26-~26: This word is normally spelled as one.
Context: ...environment (works on all shells and is super fast), requires you to use a [NerdFont](http...

(EN_COMPOUNDS_SUPER_FAST)


[style] ~27-~27: Consider shortening or rephrasing this to strengthen your wording.
Context: ...o.com) : IDE to manage the database and make changes to the source code.

  • [Remote-Contain...

(MAKE_CHANGES)


[grammar] ~29-~29: The word “setup” is a noun. The verb is spelled with a space.
Context: ...ension for VS Code. It allows to easily setup the dev environnement.

  • [SQLTools...

(NOUN_VERB_CONFUSION)


[grammar] ~42-~42: Did you mean “using”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ... environment using Docker, as it allows to use a preconfigured developement environnem...

(ALLOW_TO)


[uncategorized] ~48-~48: Loose punctuation mark.
Context: ...eate 10 containers :

  • graasp-core : Node.js backend of Graasp
  • db : Pos...

(UNLIKELY_OPENING_PUNCTUATION)


[grammar] ~53-~53: Did you mean to write ‘used to’?
Context: ...raasp-localstack` : Localstack instance use to fake S3 storage locally

  • `localfil...

(USE_TO_NP)


[formatting] ~61-~61: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...un this command in the docker's terminal, because some packages are built depending on th...

(COMMA_BEFORE_BECAUSE)


[uncategorized] ~61-~61: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...uilt depending on the operating system (eg. bcrypt).

Info
If the proc...

(E_G)


[uncategorized] ~67-~67: Possible missing comma found.
Context: ...(2 GB) to 8GB.

Once the packages are installed we need to bootstrap the database. Run ...

(AI_HYDRA_LEO_MISSING_COMMA)


[grammar] ~76-~76: Did you mean “Google Analytics”?
Context: ...raasp: the db for the backend

  • Umami: google analytics replacement
  • Etherpad: realtime docum...

(GOOGLE_PRODUCTS)


[uncategorized] ~81-~81: Possible missing comma found.
Context: ...ring tests

Local Installation

First a running and accessible instance of Po...

(AI_HYDRA_LEO_MISSING_COMMA)


[misspelling] ~252-~252: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...ttps://mailcatcher.me/), which emulates a SMTP server for sending e-mails. When u...

(EN_A_VS_AN)


[misspelling] ~254-~254: The word ‘informations’ is a legal term. In standard English, the word ‘information’ is a non-count noun.
Context: ...issues section](#known-issues) for more informations). The server is available at `http://lo...

(INFORMATIONS)


[uncategorized] ~268-~268: Possible missing comma found.
Context: ...ey will clean the database between test runs you will not loose your development dat...

(AI_HYDRA_LEO_MISSING_COMMA)


[misspelling] ~268-~268: Did you mean “lose” (= miss, waste, suffer the loss etc.)?
Context: ...database between test runs you will not loose your development data.

It's also imp...

(LOOSE_LOSE)


[grammar] ~280-~280: The verb ‘generate’ does not usually follow articles like ‘the’. Check that ‘generate’ is spelled correctly; using ‘generate’ as a noun may be non-standard.
Context: ...s files so typeorm can run them.

Run the generate and run command to create and apply the...

(A_INFINITIVE)


[typographical] ~312-~312: After an expression of time at the start of a sentence, a comma is usually used.
Context: ...tored on the fake s3 are not.

In the future we might investigate different solution...

(IN_THE_PAST_COMMA)


[style] ~312-~312: Consider an alternative to strengthen your wording.
Context: ....

In the future we might investigate different solutions to mocking s3 storage, or improve the l...

(DIFFERENT_ALTERNATIVE)


[grammar] ~322-~322: The word ‘lint’ is a noun or an adjective. A verb is missing or misspelled, or maybe a comma is missing.
Context: ...``sh
yarn openapi:generate


You can then lint the specifications with

```sh
yarn ...

(PRP_MD_NN)

</details>
<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>

3-3: No empty links
null

(MD042, no-empty-links)

---

6-6: Images should have alternate text (alt text)
null

(MD045, no-alt-text)

---

240-240: Bare URL used
null

(MD034, no-bare-urls)

</details>

</details>

</details>
<details>
<summary>src/fastify.ts (3)</summary>

`8-8`: **LGTM! Import cleanup.**

The simplified imports maintain only the necessary constants that are actively used in the code.

---

`45-45`: **LGTM! Improved code formatting.**

The added empty line enhances readability by clearly separating the server startup from the subsequent logging statements.

---

`14-14`: <details>
<summary>❓ Verification inconclusive</summary>

**Verify if 25MB body limit is sufficient for your use case.**

The increased body limit of 25MB is a good improvement over Fastify's default 1MB limit, which could have been blocking file uploads. However, please verify that this limit aligns with:
1. Your maximum expected file upload size
2. Any corresponding limits in your AWS configuration (e.g., ALB, API Gateway)
3. Memory constraints of your ECS tasks



---

🌐 Web query:

```web
What is the recommended maximum request body size limit for Fastify in production?

💡 Result:

In Fastify, request body size limits should be carefully configured based on your application's needs and security requirements:

Core Configuration

  • Global Limit: Set via bodyLimit during server creation
const fastify = require('fastify')({
  bodyLimit: 1048576 // 1 MiB (default)
})
  • JSON-specific Limit: Configure using jsonBodyLimit
const fastify = require('fastify')({
  jsonBodyLimit: 524288 // 512 KiB (example)
})

Security Recommendations

  1. Set explicit limits rather than relying on defaults
  2. Balance between functionality (expected payload sizes) and DoS protection
  3. Differentiate limits by content type using:
    • bodyLimit for generic payloads
    • jsonBodyLimit for JSON data
    • Plugin-specific limits for multipart forms

Multipart Form Handling

When using @fastify/multipart:

fastify.register(require('@fastify/multipart'), {
  limits: {
    fileSize: 10485760 // 10 MiB example
    // Other security constraints:
    fields: 15,
    files: 5
  }
})

Production Considerations

  • Start with 1-5MB for most APIs[1][7]
  • Increase for file upload endpoints (10-100MB) with proper validation[3]
  • Monitor memory usage and adjust accordingly[4][10]
  • Use streaming for very large payloads (>100MB)[1]

All limits should be explicitly set in code rather than relying on framework defaults[1][7][10]. The exact values depend on your specific use case and infrastructure capacity[4][9].

Citations:


Fastify Body Limit Validation

The increase from the default 1MB to 25MB is a solid improvement for handling file uploads. Based on production guidelines—which often recommend limits ranging from 10MB to 100MB for file upload endpoints—25MB is within an acceptable range. However, please verify that this value adequately meets your application's requirements by ensuring:

  • It matches the maximum expected file upload size.
  • It is consistent with any AWS configuration limits (e.g., ALB, API Gateway).
  • It fits within the memory constraints for your ECS tasks.
.github/workflows/test.yml (2)

61-118: LGTM! Well-structured build job with proper caching and database setup.

The build job includes:

  • PostgreSQL service container with health checks
  • Yarn dependency caching
  • Migration checks to ensure database schema consistency

54-54:

✅ Verification successful

Verify Stripe integration configuration.

The Stripe secret key is properly stored in GitHub secrets. However, ensure that this is a test key and not a production key, as this is a test workflow.


🌐 Web query:

How to identify Stripe test and production API keys?

💡 Result:

Stripe API keys are identified by their prefixes and environment modes (test vs. live). Here's how to distinguish them:

Key Prefixes

  • Test Publishable Key: Starts with pk_test_
  • Test Secret Key: Starts with sk_test_
  • Live Publishable Key: Starts with pk_live_
  • Live Secret Key: Starts with sk_live_

This prefix system applies universally across Stripe accounts[5][9].

Environment Mode

Stripe separates API keys into two environments:

Environment Purpose Key Types
Test Mode Simulate transactions without real money[1][6] pk_test_, sk_test_
Live Mode Process actual payments[3][6] pk_live_, sk_live_

How to Locate Them

  1. Log into your Stripe Dashboard
  2. Navigate to Developers → API Keys[1][3][4]
  3. Toggle between environments:
    • Use "Viewing test data" for test keys[2][3]
    • Disable test mode toggle for live keys[4][9]

Important Security Notes:

  • Live secret keys can only be revealed once during creation[3][9]
  • Test secret keys can be revealed multiple times[3]
  • Never share live secret keys publicly[5][9]

Objects created in test mode (e.g., customers, payments) don't appear in live mode, and vice versa[3][6]. Always verify you're using the correct environment before processing transactions.

Citations:


Verify Stripe Secret Key Configuration
The STRIPE_SECRET_KEY is securely stored using GitHub secrets. Since this workflow is for testing, please confirm that the secret key in use is a Stripe test key. Test secret keys typically start with sk_test_ (as opposed to production keys, which start with sk_live_). Verify in your Stripe Dashboard that the key conforms to the test mode requirements.

.github/workflows/deploy-stage.yml (1)

38-40:

❓ Verification inconclusive

Address TODO comments for AWS credentials.

The TODO comments indicate that AWS credentials need to be moved to environments. This should be prioritized to ensure proper credential management.


🏁 Script executed:

#!/bin/bash
# Check if AWS credentials are already configured in GitHub environments
gh api -H "Accept: application/vnd.github+json" /repos/lnco-epfl/graasp/environments/staging/variables | jq '.[] | select(.name | contains("AWS_"))'

Length of output: 240


Action Required: Validate AWS Credentials Management in GitHub Environments

The deployment workflow at .github/workflows/deploy-stage.yml (lines 38–40) still uses secret references for AWS credentials along with TODO comments indicating that these should be moved to environments. However, our initial verification using the GitHub API returned a 404 error—suggesting that the expected staging environment (or its AWS variables) might not be configured.

Please:

  • Manually verify that the GitHub staging environment exists and that its variables include AWS credentials (i.e., AWS_ACCESS_KEY_ID_STAGE and AWS_SECRET_ACCESS_KEY_STAGE).
  • If the environment is not set up, either create it and migrate these credentials or update/remove the TODO comments accordingly to reflect your intended credential management strategy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant