-
Notifications
You must be signed in to change notification settings - Fork 0
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
4 5 6 7 remove unnecessary back end features #12
4 5 6 7 remove unnecessary back end features #12
Conversation
…ct-permissions Update graasp-dev.json for deployment with correct IAM
…g-var Update deploy-dev.yml add h5p-storage-root-path
Remove pino-pretty in server.ts because of transport issue
WalkthroughThe changes encompass a variety of modifications across multiple files, primarily focusing on commenting out existing code, restructuring import statements, and making formatting adjustments. Key functionalities related to services such as Etherpad and H5P have been completely removed, affecting their integration and operational capabilities within the application. Additionally, tests related to search functionalities have been removed, and documentation has been streamlined for clarity, while configuration files have been enhanced for better readability. Changes
Poem
Tip OpenAI O1 model for chat
Recent review detailsConfiguration used: CodeRabbit UI Files ignored due to path filters (1)
Files selected for processing (7)
Files not reviewed due to no reviewable changes (5)
Files skipped from review as they are similar to previous changes (1)
Additional comments not posted (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Outside diff range and nitpick comments (4)
src/services/websockets/API.md (1)
101-102
: Improve clarity by separating "otherwise" from the sentence.The word "otherwise" is an adverb that can't be used like a conjunction. For better clarity, consider separating it from the sentence.
Apply this change:
- the `error` field is optional: if and only if the `<responseStatus>` is `error`, then the `error` field is populated, otherwise it is `undefined`. The error has a name and a descriptive message + the `error` field is optional: if and only if the `<responseStatus>` is `error`, then the `error` field is populated. Otherwise, it is `undefined`. The error has a name and a descriptive messageTools
LanguageTool
[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
errorfield is populated, otherwise it is
undefined`. The error has a name...(THUS_SENTENCE)
src/utils/config.ts (2)
Line range hint
281-337
: Remove the commented out code block.The large commented out code block related to H5P and Etherpad configuration should be removed completely instead of being left as a comment. Removing unused code helps keep the codebase clean and maintainable.
Consider applying this change:
-/* -// H5P and Etherpad Initialization -if (!process.env.H5P_PATH_PREFIX) { - throw new Error('Invalid H5P path prefix'); -} -export const H5P_PATH_PREFIX = process.env.H5P_PATH_PREFIX; - -// ugly runtime type checking since typescript cannot infer types -// todo: please use a typed env checker library, this is awful -if ( - process.env.H5P_FILE_STORAGE_TYPE !== ItemType.S3_FILE && - process.env.H5P_FILE_STORAGE_TYPE !== ItemType.LOCAL_FILE -) { - throw new Error('Invalid H5P file storage type provided'); -} -export const H5P_FILE_STORAGE_TYPE = process.env.H5P_FILE_STORAGE_TYPE as FileItemType; - -// ugly runtime type checking since typescript cannot infer types -if (H5P_FILE_STORAGE_TYPE === ItemType.S3_FILE) { - if ( - !process.env.H5P_CONTENT_REGION || - !process.env.H5P_CONTENT_BUCKET || - !process.env.H5P_CONTENT_SECRET_ACCESS_KEY_ID || - !process.env.H5P_CONTENT_ACCESS_KEY_ID - ) - throw new Error('H5P S3 configuration missing'); -} -export const H5P_S3_CONFIG = { - s3: { - s3Region: process.env.H5P_CONTENT_REGION, - s3Bucket: process.env.H5P_CONTENT_BUCKET, - s3SecretAccessKey: process.env.H5P_CONTENT_SECRET_ACCESS_KEY_ID, - s3AccessKeyId: process.env.H5P_CONTENT_ACCESS_KEY_ID, - } as S3FileConfiguration, -}; - -// ugly runtime type checking since typescript cannot infer types -if (H5P_FILE_STORAGE_TYPE === ItemType.LOCAL_FILE) { - if (!process.env.H5P_STORAGE_ROOT_PATH) throw new Error('H5P local storage root path missing'); -} -export const H5P_LOCAL_CONFIG = { - local: { - storageRootPath: process.env.H5P_STORAGE_ROOT_PATH, - localFilesHost: process.env.H5P_FILE_STORAGE_HOST, - } as LocalFileConfiguration, -}; - -// ugly runtime type checking since typescript cannot infer types -export const H5P_FILE_STORAGE_CONFIG = - H5P_FILE_STORAGE_TYPE === ItemType.S3_FILE ? H5P_S3_CONFIG : H5P_LOCAL_CONFIG; - -export const ETHERPAD_URL = validateEnv('ETHERPAD_URL', new UrlValidator()); - -export const ETHERPAD_PUBLIC_URL = process.env.ETHERPAD_PUBLIC_URL; -export const ETHERPAD_API_KEY = validateEnv('ETHERPAD_API_KEY', new RegexValidator(API_KEY_FORMAT)); -export const ETHERPAD_COOKIE_DOMAIN = process.env.ETHERPAD_COOKIE_DOMAIN; -*/
399-406
: Remove the commented out code block.The commented out code block related to Meilisearch configuration should be removed completely instead of being left as a comment. Removing unused code helps keep the codebase clean and maintainable.
Consider applying this change:
-/* -// Graasp Search -export const MEILISEARCH_URL = process.env.MEILISEARCH_URL || ''; -export const MEILISEARCH_MASTER_KEY = process.env.MEILISEARCH_MASTER_KEY; -export const MEILISEARCH_REBUILD_SECRET = process.env.MEILISEARCH_REBUILD_SECRET; -export const MEILISEARCH_STORE_LEGACY_PDF_CONTENT: boolean = - process.env.MEILISEARCH_STORE_LEGACY_PDF_CONTENT === 'true'; -*/src/services/item/plugins/publication/published/plugins/search/meilisearch.ts (1)
1-452
: Removal of MeiliSearch integration code approved.The entire
meilisearch.ts
file has been commented out, effectively disabling the MeiliSearch integration. This aligns with the PR objective of removing unnecessary back-end features.If the removal of MeiliSearch is intended to be permanent, consider deleting this file entirely to keep the codebase clean. If there's a chance it might be needed in the future, it's fine to keep it commented out as is.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
Files selected for processing (57)
- .aws/graasp-prod.json (1 hunks)
- .husky/commit-msg (1 hunks)
- .husky/install.mjs (1 hunks)
- .prettierrc (1 hunks)
- .yarnrc.yml (1 hunks)
- API Collections/bruno.json (1 hunks)
- docs/architecture.md (2 hunks)
- docs/meilisearch.md (1 hunks)
- package.json (1 hunks)
- src/di/container.ts (7 hunks)
- src/jobs.ts (2 hunks)
- src/plugins/datasource.ts (3 hunks)
- src/plugins/meta.ts (6 hunks)
- src/server.ts (2 hunks)
- src/services/item/index.ts (4 hunks)
- src/services/item/plugins/etherpad/constants.ts (1 hunks)
- src/services/item/plugins/etherpad/errors.ts (1 hunks)
- src/services/item/plugins/etherpad/etherpad.ts (1 hunks)
- src/services/item/plugins/etherpad/index.ts (1 hunks)
- src/services/item/plugins/etherpad/schemas.ts (1 hunks)
- src/services/item/plugins/etherpad/service-api.ts (1 hunks)
- src/services/item/plugins/etherpad/service.ts (13 hunks)
- src/services/item/plugins/etherpad/serviceConfig.ts (2 hunks)
- src/services/item/plugins/etherpad/test/api.ts (3 hunks)
- src/services/item/plugins/etherpad/test/index.test.ts (1 hunks)
- src/services/item/plugins/etherpad/test/service.test.ts (1 hunks)
- src/services/item/plugins/etherpad/types.ts (1 hunks)
- src/services/item/plugins/html/constants.ts (1 hunks)
- src/services/item/plugins/html/h5p/constants.ts (1 hunks)
- src/services/item/plugins/html/h5p/errors.ts (1 hunks)
- src/services/item/plugins/html/h5p/index.ts (1 hunks)
- src/services/item/plugins/html/h5p/integration.ts (1 hunks)
- src/services/item/plugins/html/h5p/schemas.ts (1 hunks)
- src/services/item/plugins/html/h5p/service.ts (1 hunks)
- src/services/item/plugins/html/h5p/test/errors.test.ts (1 hunks)
- src/services/item/plugins/html/h5p/test/fixtures.ts (1 hunks)
- src/services/item/plugins/html/h5p/test/h5p/h5p-validator.test.ts (1 hunks)
- src/services/item/plugins/html/h5p/test/helpers.ts (1 hunks)
- src/services/item/plugins/html/h5p/test/plugin.test.ts (1 hunks)
- src/services/item/plugins/html/h5p/types.ts (1 hunks)
- src/services/item/plugins/html/h5p/validation/h5p-validator.ts (1 hunks)
- src/services/item/plugins/html/h5p/validation/h5p.ts (1 hunks)
- src/services/item/plugins/html/service.ts (1 hunks)
- src/services/item/plugins/importExport/service.ts (4 hunks)
- src/services/item/plugins/invitation/specifications.md (4 hunks)
- src/services/item/plugins/publication/published/index.ts (2 hunks)
- src/services/item/plugins/publication/published/plugins/search/index.ts (1 hunks)
- src/services/item/plugins/publication/published/plugins/search/meilisearch.ts (1 hunks)
- src/services/item/plugins/publication/published/plugins/search/schemas.ts (1 hunks)
- src/services/item/plugins/publication/published/plugins/search/service.ts (1 hunks)
- src/services/item/plugins/publication/published/plugins/search/types.ts (1 hunks)
- src/services/item/plugins/publication/published/test/meilisearch.test.ts (1 hunks)
- src/services/item/test/fixtures/items.ts (5 hunks)
- src/services/itemMembership/specifications.md (2 hunks)
- src/services/websockets/API.md (1 hunks)
- src/services/websockets/USAGE.md (1 hunks)
- src/utils/config.ts (5 hunks)
Files skipped from review due to trivial changes (33)
- .aws/graasp-prod.json
- .husky/commit-msg
- .husky/install.mjs
- .prettierrc
- .yarnrc.yml
- API Collections/bruno.json
- src/jobs.ts
- src/services/item/plugins/etherpad/constants.ts
- src/services/item/plugins/etherpad/errors.ts
- src/services/item/plugins/etherpad/etherpad.ts
- src/services/item/plugins/etherpad/schemas.ts
- src/services/item/plugins/etherpad/service.ts
- src/services/item/plugins/etherpad/serviceConfig.ts
- src/services/item/plugins/etherpad/test/api.ts
- src/services/item/plugins/etherpad/test/service.test.ts
- src/services/item/plugins/etherpad/types.ts
- src/services/item/plugins/html/constants.ts
- src/services/item/plugins/html/h5p/errors.ts
- src/services/item/plugins/html/h5p/index.ts
- src/services/item/plugins/html/h5p/service.ts
- src/services/item/plugins/html/h5p/test/errors.test.ts
- src/services/item/plugins/html/h5p/test/h5p/h5p-validator.test.ts
- src/services/item/plugins/html/h5p/test/plugin.test.ts
- src/services/item/plugins/html/h5p/validation/h5p-validator.ts
- src/services/item/plugins/html/h5p/validation/h5p.ts
- src/services/item/plugins/html/service.ts
- src/services/item/plugins/publication/published/index.ts
- src/services/item/plugins/publication/published/plugins/search/index.ts
- src/services/item/plugins/publication/published/plugins/search/schemas.ts
- src/services/item/plugins/publication/published/plugins/search/service.ts
- src/services/item/plugins/publication/published/test/meilisearch.test.ts
- src/services/itemMembership/specifications.md
- src/services/websockets/USAGE.md
Additional context used
LanguageTool
docs/architecture.md
[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 po...(OUTSIDE_OF)
docs/meilisearch.md
[uncategorized] ~30-~30: Loose punctuation mark.
Context: ...ctory. ## Modules -meilisearch.ts
: Abstracts Meilisearch usage for the r...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~31-~31: Loose punctuation mark.
Context: ...ing the indexing logic. -service.ts
: Handles search queries and sets up ho...(UNLIKELY_OPENING_PUNCTUATION)
src/services/item/plugins/invitation/specifications.md
[style] ~34-~34: Consider an alternative for the often overused word ‘important’.
Context: ...NOTES**: - Order of the columns is not important. - Spaces around the delimiter are supp...(NOT_IMPORTANT)
src/services/websockets/API.md
[grammar] ~24-~24: There seems to be a noun/verb agreement error. Did you mean “subscribes” or “subscribed”?
Context: ...ne 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
errorfield is populated, otherwise it is
undefined`. The error has a name...(THUS_SENTENCE)
[uncategorized] ~106-~106: Loose punctuation mark.
Context: ...r messages include: -ACCESS_DENIED
: the client is not allowed to access the...(UNLIKELY_OPENING_PUNCTUATION)
Gitleaks
src/services/item/plugins/etherpad/test/index.test.ts
32-32: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
Additional comments not posted (48)
src/services/item/plugins/publication/published/plugins/search/types.ts (1)
1-6
: LGTM, but verify the enum usage.The change is approved as it aligns with the PR objective of removing unnecessary back-end features.
However, ensure that the
Ranges
enum is not used anywhere else in the codebase. If it is used, this change may lead to compilation errors or runtime errors.Run the following script to verify the enum usage:
Verification successful
Verification Successful:
Ranges
Enum Not Used ElsewhereThe
Ranges
enum, which was commented out, is not used anywhere else in the codebase. This confirms that the change does not affect other parts of the codebase and aligns with the PR objective. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `Ranges` enum is not used anywhere else in the codebase. # Test: Search for the enum usage. Expect: No occurrences. rg --type typescript $'Ranges\\.'Length of output: 68
Script:
#!/bin/bash # Description: Verify that the `Ranges` enum is not used anywhere else in the codebase. # Test: Search for the enum usage. Expect: No occurrences. rg 'Ranges\.'Length of output: 15
src/services/item/plugins/html/h5p/types.ts (1)
1-12
: LGTM! The changes align with the PR objective.The entire
H5PPluginOptions
interface has been commented out, effectively removing it from active use. This change is consistent with the PR objective of removing H5P features from the back-end.It's important to note that:
- Any functionality relying on
H5PPluginOptions
will no longer be available.- The front-end features related to H5P are still present and will be addressed separately, as mentioned in the PR summary.
src/services/item/plugins/html/h5p/constants.ts (1)
1-22
: LGTM, but verify the impact on dependent code.The changes to comment out the entire file are approved, as they align with the PR objective of removing unnecessary back-end features related to H5P.
However, ensure that all code that depends on these constants is properly handled. This might involve removing or updating the dependent code, or adding appropriate checks to handle the absence of these constants.
Run the following script to verify the usage of the exported constants:
Verification successful
Verification successful: No impact on dependent code.
The constants from
src/services/item/plugins/html/h5p/constants.ts
are not used elsewhere in the codebase, confirming that commenting them out does not affect other parts of the codebase. This change aligns with the PR objective of removing unnecessary back-end features related to H5P. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the exported constants from `src/services/item/plugins/html/h5p/constants.ts`. # Test: Search for the usage of the exported constants. Expect: No occurrences or only occurrences in commented out code. rg --type typescript -A 5 $'MAX_FILE_SIZE|MAX_NON_FILE_FIELDS|MAX_FILES|H5P_FILE_MIME_TYPE|H5P_FILE_DOT_EXTENSION|DEFAULT_MIME_TYPE|DEFAULT_H5P_CONTENT_ROUTE|DEFAULT_H5P_ASSETS_ROUTE|PLUGIN_NAME'Length of output: 231
Script:
#!/bin/bash # Description: Verify the usage of the exported constants from `src/services/item/plugins/html/h5p/constants.ts`. # Test: Search for the usage of the exported constants. Expect: No occurrences or only occurrences in commented out code. rg --type ts -A 5 $'MAX_FILE_SIZE|MAX_NON_FILE_FIELDS|MAX_FILES|H5P_FILE_MIME_TYPE|H5P_FILE_DOT_EXTENSION|DEFAULT_MIME_TYPE|DEFAULT_H5P_CONTENT_ROUTE|DEFAULT_H5P_ASSETS_ROUTE|PLUGIN_NAME'Length of output: 36565
docs/architecture.md (1)
31-32
: LGTM!The new section "Important Utils" and the description of the
validatePermission
function are clear and informative. The changes are approved.src/server.ts (1)
8-8
: Verify the impact of commenting outAPP_VERSION
on logging and monitoring.The
APP_VERSION
constant has been commented out, which removes its usage in the logging statement that previously reported the application version.This change may reduce the information available for monitoring the application's performance and health.
Run the following script to verify the impact on logging and monitoring:
Verification successful
Partial impact on logging and monitoring due to
APP_VERSION
being commented out.While
APP_VERSION
has been commented out insrc/server.ts
, it is still used in other parts of the codebase, such assrc/sentry.ts
for release tracking andsrc/plugins/swagger.ts
for documentation purposes. This indicates that the application's version information is still being utilized, albeit not in the logging statement withinsrc/server.ts
.
src/sentry.ts
: UsesAPP_VERSION
for Sentry release tracking.src/plugins/swagger.ts
: UsesAPP_VERSION
for Swagger documentation.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the application version is still being logged after commenting out `APP_VERSION`. # Test: Search for the usage of `APP_VERSION` in logging statements. Expect: No occurrences found. rg --type typescript $'APP_VERSION'Length of output: 69
Script:
#!/bin/bash # Description: Verify if the application version is still being logged after commenting out `APP_VERSION`. # Test: Search for the usage of `APP_VERSION` in all files. Expect: No occurrences found. rg 'APP_VERSION'Length of output: 599
docs/meilisearch.md (1)
Line range hint
1-45
: Documentation changes look good!The formatting adjustments and minor content enhancements improve the document's readability without altering its core content or functionality. The changes are approved.
Tools
LanguageTool
[uncategorized] ~30-~30: Loose punctuation mark.
Context: ...ctory. ## Modules -meilisearch.ts
: Abstracts Meilisearch usage for the r...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~31-~31: Loose punctuation mark.
Context: ...ing the indexing logic. -service.ts
: Handles search queries and sets up ho...(UNLIKELY_OPENING_PUNCTUATION)
src/services/item/plugins/html/h5p/integration.ts (1)
1-59
: LGTM! The changes align with the PR objective.The entire
renderHtml
function and its associated documentation have been commented out, effectively disabling the functionality of rendering H5P integration HTML for the Graasp platform. This change is consistent with the PR objective of removing unnecessary back-end features, specifically H5P in this case.The code changes are approved.
src/services/item/plugins/html/h5p/test/helpers.ts (1)
1-74
: Verify the intent behind commenting out the entire file.The entire
helpers.ts
file has been commented out, which effectively disables all the functionality provided by this module, including theinjectH5PImport
andexpectH5PFiles
functions.This change aligns with the PR objective of removing H5P-related features. However, without additional context, the specific intent behind this change remains unclear.
Please confirm if the intent is to permanently remove this file and its associated tests. If so, consider deleting the file instead of commenting it out.
src/services/item/plugins/invitation/specifications.md (4)
9-11
: LGTM!The formatting changes to the route and params sections improve the readability of the documentation.
25-31
: LGTM!The formatting changes to the CSV file specifications table enhance the clarity and usability of the documentation.
32-32
: LGTM!The formatting changes to the notes section improve the readability of the documentation.
The LanguageTool hint about the word "important" being overused is a false positive and can be ignored in this context.
40-40
: LGTM!The changes to the valid CSV files and error handling sections enhance the clarity and usability of the documentation by providing better examples and error reasons.
Also applies to: 47-47, 55-55, 63-63, 71-71, 82-82, 96-96
src/services/item/plugins/etherpad/service-api.ts (1)
1-97
: Approved: The changes align with the PR objective of removing the Etherpad feature.Commenting out the entire file effectively disables the Etherpad functionality, which is no longer needed as per the PR description. This change represents a substantial modification that removes the operational capabilities of the Etherpad service.
Some key observations:
- The exported Fastify plugin and its associated routes for creating and viewing Etherpad items are now inactive.
- The hooks for deleting and copying Etherpads associated with items are also disabled.
- Any part of the application that attempts to invoke the commented-out functionality may lead to runtime errors.
Since the decision to remove this feature was already made and communicated in the PR summary, and the code changes align with that objective, this can be approved.
src/plugins/datasource.ts (4)
23-23
: LGTM!The changes are approved.
24-26
: LGTM!The changes are approved.
92-92
: LGTM!The changes are approved.
101-103
: LGTM!The changes are approved.
src/plugins/meta.ts (6)
8-12
: LGTM!The commented out import statements align with the PR objective of removing unnecessary back-end features.
74-74
: LGTM!The commented out code aligns with the PR objective of removing unnecessary back-end features.
77-78
: LGTM!The commented out code aligns with the PR objective of removing unnecessary back-end features.
86-87
: LGTM!The commented out code aligns with the PR objective of removing unnecessary back-end features.
Line range hint
111-128
: LGTM!The commented out function aligns with the PR objective of removing unnecessary back-end features.
Line range hint
147-162
: LGTM!The commented out function aligns with the PR objective of removing unnecessary back-end features.
src/services/item/index.ts (5)
30-30
: LGTM!The code change is approved.
33-33
: LGTM!The code change is approved.
41-43
: LGTM!The code changes are approved.
90-90
: LGTM!The code changes are approved.
Also applies to: 109-109, 111-111
129-129
: LGTM!The code change is approved.
src/di/container.ts (5)
3-3
: LGTM!The commented-out import statements align with the PR objective of removing unnecessary back-end features.
Also applies to: 6-7, 10-10, 12-17, 19-19, 22-23
37-38
: LGTM!The commented-out constants align with the PR objective of removing unnecessary back-end features.
Also applies to: 47-47
Line range hint
102-121
: LGTM!The commented-out code block for MeiliSearch registration aligns with the PR objective of removing unnecessary back-end features.
Line range hint
123-132
: LGTM!The commented-out code block for job workers aligns with the PR objective of removing unnecessary back-end features.
Line range hint
134-151
: LGTM!The commented-out code block for Etherpad registration aligns with the PR objective of removing unnecessary back-end features.
package.json (2)
153-153
: LGTM!The addition of the
commitlint
dependency is approved. It will help maintain a consistent commit message format in the project.
160-160
: Verify compatibility with the existing scripts and configuration.The upgrade of the
husky
dependency to version9.1.5
is approved.However, since this is a major version upgrade, ensure that the new version is compatible with the existing scripts and configuration defined in the project.
Run the following script to verify the
husky
setup:src/services/websockets/API.md (1)
14-19
: JSON formatting changes look good!The addition of trailing commas to the JSON message structures enhances readability and aligns with common JSON formatting practices. The changes maintain consistency across the examples without altering the semantic meaning of the API documentation.
Also applies to: 26-33, 42-49, 55-62, 70-75, 82-96, 113-123, 128-135
src/services/item/plugins/html/h5p/schemas.ts (1)
1-315
: The changes are consistent with the PR objective. Approved.The entire file has been commented out, which effectively disables the H5P functionality. This change is consistent with the PR objective of removing H5P features.
Commenting out the code instead of deleting it allows for easier reversion if needed in the future.
src/services/item/test/fixtures/items.ts (6)
5-6
: LGTM!The code changes are approved.
26-26
: LGTM!The code changes are approved.
35-35
: LGTM!The code changes are approved.
42-42
: LGTM!The code changes are approved.
72-77
: LGTM!The code changes are approved.
202-202
: LGTM!The code changes are approved.
src/services/item/plugins/importExport/service.ts (4)
23-23
: LGTM!The commented out import statement for
H5PService
aligns with the PR objective of removing H5P related features.
40-40
: LGTM!The commented out private member declaration for
h5pService
aligns with the PR objective of removing H5P related features.
49-49
: LGTM!The commented out constructor parameter for
h5pService
aligns with the PR objective of removing H5P related features.
231-237
: LGTM!The commented out case statement for handling items of type
H5P
aligns with the PR objective of removing H5P related features.src/services/item/plugins/etherpad/test/index.test.ts (1)
32-32
: Gitleaks warning is a false positive. The flagged line does not contain sensitive information.The line flagged by Gitleaks does not appear to contain an actual API key upon examination. It is constructing a mock pad ID using mock values (
MOCK_GROUP_ID
andMOCK_PAD_NAME
) for the tests.These mock constants are used throughout the test suite to provide dummy values. They do not represent real API keys or sensitive information.
Therefore, the Gitleaks warning can be considered a false positive in this case.
Tools
Gitleaks
32-32: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
src/server.ts
Outdated
// instance.log.info('App is running version %s in %s mode', APP_VERSION, ENVIRONMENT); | ||
if (DEV) { | ||
// greet the world | ||
console.log(`${GREETING}`); | ||
} | ||
} catch (err) { | ||
instance.log.error(err); | ||
// instance.log.error(err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider the potential impact of commenting out logging statements on monitoring and error reporting.
The following logging statements have been commented out:
- The logging statement that reported the application version and environment mode.
- The error logging line.
These changes suggest a shift towards a more minimal logging approach, possibly for debugging or testing purposes, but they may reduce the information available for monitoring the application's performance and health.
Commenting out the logging statement that reported the application version and environment mode reduces the visibility of the application's operational context.
Commenting out the error logging line may hinder the ability to capture and report errors effectively.
Consider reverting these changes or providing an alternative mechanism for logging and error reporting to ensure effective monitoring and debugging capabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-enabled this in a new commit with console.log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-enabled this in a new commit with console.log instead of instance.log. Changed because of "pino-pretty".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymijsters Thank you for addressing the logging concern. I've reviewed the latest changes, and I have a few observations and suggestions:
-
The console.log for the greeting in DEV mode is indeed present and uncommented. However, the main application logging (version, environment info, and error logging) is still commented out.
-
The transition from instance.log to console.log is not fully implemented as described. The instance.log.info and instance.log.error statements are still present but commented out.
-
I noticed that pino-pretty is still listed as a dependency in your package.json and yarn.lock files, but its configuration in server.ts is commented out.
To ensure consistency and maintain effective logging, I suggest:
-
Complete the transition to console.log by replacing the commented-out instance.log statements. For example:
console.log(`App is running version ${APP_VERSION} in ${ENVIRONMENT} mode`);
and
console.error(err);
-
Consider your long-term logging strategy. While console.log is simpler, it lacks some features of structured logging systems like pino. If you decide to stick with console.log, you might want to remove pino-pretty from your dependencies to keep your project lean.
-
If you plan to use more advanced logging features in the future, you might want to consider keeping a structured logging system but configuring it to use a simpler output format in development.
Could you clarify your intentions regarding the logging strategy? Are you planning to use console.log exclusively, or is this a temporary measure while you decide on a more comprehensive logging approach?
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Fetch the latest changes to server.ts
cat src/server.ts
# Check for any pino-pretty related configurations
grep -r "pino-pretty" .
Length of output: 2781
Learnings added
Learnt from: ymijsters
PR: lnco-epfl/graasp#12
File: src/server.ts:62-68
Timestamp: 2024-09-12T08:21:26.759Z
Learning: The project is transitioning from using instance.log (likely from a structured logging system) to console.log for simplicity, particularly due to issues with pino-pretty. However, the transition is not yet complete, and the long-term logging strategy is still under consideration.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymijsters Thank you for addressing the logging concern. However, I've checked the latest commit for src/server.ts, and I couldn't find any new console.log statements that replace the commented-out logging. The instance.log.info and instance.log.error statements are still commented out. The only console.log present is the existing one for GREETING, which was already there and only runs in DEV mode.
Could you please verify if the changes you mentioned were made in a different file, or if perhaps the commit hasn't been pushed yet? It's important to ensure that proper logging is in place, especially for server-side applications.
While console.log can be useful for quick debugging, it's generally not recommended for production server environments due to several reasons:
- Performance: console.log can be slower than dedicated logging libraries.
- Lack of log levels: It doesn't provide different severity levels (info, warn, error, etc.).
- No built-in log rotation or management: This can lead to issues with log file sizes.
- Asynchronous nature: In Node.js, console.log is asynchronous and can lead to out-of-order logs in high-concurrency situations.
Instead, I would recommend using a robust logging library like Winston or Pino, which are designed for server-side logging and offer features like:
- Log levels
- Structured logging
- Log rotation
- Performance optimizations
- Easy integration with log management systems
If you decide to stick with the built-in logging, consider using instance.log
as it was originally implemented, as it's likely configured to handle server logging more appropriately.
Could you provide more details about where you've implemented the new logging, or if you're planning to make these changes in a future commit?
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check the latest commit for changes related to logging
git show -p HEAD src/server.ts | grep -E "console.log|instance.log"
Length of output: 350
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you recheck?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymijsters Thank you for asking me to recheck. I've reviewed the latest version of src/server.ts
, and I can confirm that the changes we discussed have been implemented. Here's what I found:
-
The main application logging has been updated to use
console.log
:console.log('App is running version %s in %s mode', APP_VERSION, ENVIRONMENT);
-
Error logging now uses
console.error
:console.error(err);
-
The previously commented-out
instance.log
statements have been removed entirely.
These changes align with the transition from instance.log
to console.log
that we discussed earlier. The implementation looks good and addresses the concerns we had about incomplete logging.
While this approach works, I'd like to offer a few suggestions for potential future improvements:
-
Consider using a logging library like Winston or Pino in the long term. These libraries offer more features like log levels, structured logging, and better performance, which can be beneficial as your application grows.
-
If you decide to stick with
console.log
, you might want to create a simple wrapper function that adds timestamps and log levels. For example:function log(level, message, ...args) { console.log(`[${new Date().toISOString()}] ${level}: ${message}`, ...args); } // Usage log('INFO', 'App is running version %s in %s mode', APP_VERSION, ENVIRONMENT);
-
For error logging, consider including more context. For instance:
console.error('Server error:', err.message, '\n', err.stack);
These are just suggestions for future consideration. The current implementation is functional and addresses the immediate concerns we discussed.
Is there anything else you'd like me to check or any other aspects of the logging you'd like to discuss?
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Fetch the latest content of src/server.ts
cat src/server.ts
Length of output: 2345
Learnings added
Learnt from: ymijsters
PR: lnco-epfl/graasp#12
File: src/server.ts:0-0
Timestamp: 2024-09-12T13:01:44.833Z
Learning: The project has completed the transition from using instance.log to console.log for server logging in src/server.ts. The main application logging now uses console.log, and error logging uses console.error. This change was made to simplify the logging process, moving away from structured logging libraries.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
src/server.ts
Outdated
CORS_ORIGIN_REGEX, | ||
DEV, | ||
ENVIRONMENT, | ||
DEV, // ENVIRONMENT, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
The ENVIRONMENT
constant is no longer used in logging, which may impact monitoring and debugging.
The ENVIRONMENT
constant has been commented out in a logging statement in src/server.ts
, which means the application no longer logs the environment mode. This change could reduce the information available for monitoring and debugging the application's operational context.
src/server.ts
: The logging statement usingENVIRONMENT
is commented out.src/utils/config.ts
: TheENVIRONMENT
constant is still used to define other environment-related constants.
Analysis chain
Verify the impact of commenting out ENVIRONMENT
on logging and monitoring.
The ENVIRONMENT
constant has been commented out, which removes its usage in the logging statement that previously reported the environment mode.
This change may reduce the information available for monitoring the application's performance and health.
Run the following script to verify the impact on logging and monitoring:
The usage of DEV
constant looks good.
The DEV
constant is still being used in a conditional block to greet the world in development mode.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the environment mode is still being logged after commenting out `ENVIRONMENT`.
# Test: Search for the usage of `ENVIRONMENT` in logging statements. Expect: No occurrences found.
rg --type typescript $'ENVIRONMENT'
Length of output: 69
Script:
#!/bin/bash
# Description: Verify if the environment mode is still being logged after commenting out `ENVIRONMENT`.
# Test: Search for the usage of `ENVIRONMENT` in logging statements. Expect: No occurrences found.
rg --type ts $'ENVIRONMENT'
Length of output: 552
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-enabled this var because it is now used in logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymijsters Thank you for your comment about re-enabling the ENVIRONMENT
variable for logging. I've checked the current state of the code, and I noticed that the ENVIRONMENT
variable is indeed defined and used in src/utils/config.ts
for various environment checks. However, I couldn't find any active usage of ENVIRONMENT
in logging statements.
In src/server.ts
, the logging statement that uses ENVIRONMENT
is still commented out:
// instance.log.info('App is running version %s in %s mode', APP_VERSION, ENVIRONMENT);
Could you please clarify where exactly you've re-enabled ENVIRONMENT
for logging? If you intended to use it in the logging statement in src/server.ts
, you might want to uncomment that line.
If you've added a new logging statement using ENVIRONMENT
elsewhere, could you please point me to the specific file and line? This will help me understand the changes you've made and provide more accurate assistance.
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the usage of ENVIRONMENT in TypeScript files, focusing on logging statements
rg --type ts 'ENVIRONMENT.*log' src/
Length of output: 38
Script:
#!/bin/bash
# Search for any usage of ENVIRONMENT in TypeScript files
rg --type ts 'ENVIRONMENT' src/
Length of output: 557
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you recheck?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymijsters Thank you for asking me to recheck. I apologize for the confusion in my previous response. You were right to ask for a recheck, as there have indeed been changes to the usage of the ENVIRONMENT variable. Here's what I found:
-
The logging statement in
src/server.ts
is now active:console.log('App is running version %s in %s mode', APP_VERSION, ENVIRONMENT);
This line is no longer commented out, which means the application will now log the environment mode as you intended.
-
ENVIRONMENT is still imported and used in
src/server.ts
:import { APP_VERSION, CORS_ORIGIN_REGEX, DEV, ENVIRONMENT, HOSTNAME, PORT } from './utils/config';
-
In
src/utils/config.ts
, ENVIRONMENT is defined and used for various environment checks:export const ENVIRONMENT: Environment = (() => { // ... (definition not shown in the search results) })(); export const PROD = ENVIRONMENT === Environment.production; export const STAGING = ENVIRONMENT === Environment.staging; export const DEV = ENVIRONMENT === Environment.development; export const TEST = ENVIRONMENT === Environment.test;
These findings confirm that the ENVIRONMENT variable is now being used for logging as you mentioned. The application will log the running version and environment mode when it starts.
Thank you for bringing this to my attention. Is there anything else you'd like me to check or explain regarding the usage of ENVIRONMENT?
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any usage of ENVIRONMENT in TypeScript files
rg --type ts 'ENVIRONMENT' src/
# Search for ENVIRONMENT specifically in logging statements
rg --type ts 'ENVIRONMENT.*log' src/
# Check the content of src/server.ts
cat src/server.ts
Length of output: 3009
src/server.ts
Outdated
HOSTNAME, | ||
PORT, | ||
PROD, | ||
PORT, // PROD, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
The PROD
constant is still in use and should not be commented out.
The PROD
constant is actively used in several parts of the codebase, including security configurations and transport settings. Commenting it out in src/server.ts
could lead to configuration issues or runtime errors.
src/server.ts
: Used in transport settings.src/services/auth/plugins/passport/plugin.ts
: Used in security settings.
Please ensure that the PROD
constant is correctly configured and utilized where necessary.
Analysis chain
Verify the impact of commenting out PROD
constant.
The PROD
constant has been commented out.
Run the following script to verify the impact of commenting out PROD
constant:
The usage of PORT
constant looks good.
The PORT
constant is still being used to set the port number for the server to listen on.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the `PROD` constant is still being used after being commented out.
# Test: Search for the usage of `PROD` constant. Expect: No occurrences found.
rg --type typescript $'PROD'
Length of output: 62
Script:
#!/bin/bash
# Description: Verify if the `PROD` constant is still being used after being commented out.
# Test: Search for the usage of `PROD` constant across all files.
rg 'PROD'
Length of output: 287
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed PROD in the server.ts, it is still used in plugin.ts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymijsters Thank you for the update. You've correctly implemented the change by removing PROD in server.ts while maintaining its usage in plugin.ts. This aligns well with our previous recommendation.
I can confirm that PROD is still present and used appropriately in src/services/auth/plugins/passport/plugin.ts. The removal in server.ts appears to be done safely by commenting it out, which is a good approach for making such changes.
To ensure that this change doesn't affect any functionality, it would be wise to:
- Review any code in server.ts that previously relied on the PROD constant and ensure it's adjusted accordingly.
- Run your test suite, if available, to catch any potential issues.
- Consider adding a comment near the commented-out PROD in server.ts explaining why it was removed, for future reference.
If you encounter any unexpected behavior after this change, please don't hesitate to ask for further assistance.
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify PROD usage in plugin.ts
rg 'PROD' src/services/auth/plugins/passport/plugin.ts
# Check for any remaining PROD references in server.ts
rg 'PROD' src/server.ts
# Look for any potential issues caused by PROD removal in server.ts
rg 'process\.env\.NODE_ENV === (\'production\'|"production")' src/server.ts
Length of output: 371
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you recheck?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymijsters Thank you for asking me to recheck. I've analyzed the current state of the codebase regarding the PROD constant, and here's what I found:
-
In
src/server.ts
:- The PROD constant has been successfully removed from the import statement.
- There is still a reference to PROD, but it's within a commented-out block (lines 15-23). This doesn't affect the functionality but could be cleaned up for better code hygiene.
-
In
src/services/auth/plugins/passport/plugin.ts
:- PROD is still being used, which is consistent with our previous discussion and your intention.
Overall, it appears that you've implemented the changes correctly. The PROD constant has been effectively removed from active use in server.ts while maintaining its usage in the passport plugin.
To further improve the code, you might consider:
-
Removing the commented-out block in server.ts that still references PROD (lines 15-23), as it's no longer needed and could cause confusion in the future.
-
Running a comprehensive test suite, if available, to ensure that the removal of PROD from server.ts hasn't introduced any unexpected behavior in your application.
Is there anything else you'd like me to check or explain regarding these changes?
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Recheck the changes in server.ts and PROD usage
# 1. Examine the current content of server.ts
echo "Content of server.ts:"
cat src/server.ts
# 2. Look for any remaining PROD references in server.ts
echo -e "\nPROD references in server.ts:"
rg 'PROD' src/server.ts
# 3. Check the usage of PROD in plugin.ts
echo -e "\nPROD usage in src/services/auth/plugins/passport/plugin.ts:"
rg 'PROD' src/services/auth/plugins/passport/plugin.ts
# 4. Look for any potential issues caused by PROD removal in server.ts
echo -e "\nPotential issues in server.ts:"
rg 'process\.env\.NODE_ENV === (\'production\'|"production")' src/server.ts
# 5. Check if PROD is still imported in server.ts
echo -e "\nPROD import in server.ts:"
rg 'import.*PROD' src/server.ts
Length of output: 3042
// // create an existing etherpad item to test hooks | ||
// setUpApi({ | ||
// createGroupIfNotExistsFor: [ | ||
// StatusCodes.OK, | ||
// { code: 0, message: 'ok', data: { groupID: MOCK_GROUP_ID } }, | ||
// ], | ||
// createGroupPad: [StatusCodes.OK, { code: 0, message: 'ok', data: null }], | ||
// }); | ||
// const res = await app.inject(payloadCreate); | ||
// expect(res.statusCode).toBe(StatusCodes.OK); | ||
// item = res.json(); | ||
// cleanAll(); | ||
// }); | ||
|
||
// it('deletes pad when item is deleted', async () => { | ||
// const reqsParams = setUpApi({ | ||
// deletePad: [StatusCodes.OK, { code: 0, message: 'ok', data: null }], | ||
// }); | ||
// await app.inject({ | ||
// method: 'DELETE', | ||
// url: `/items/`, | ||
// query: { | ||
// id: [item.id], | ||
// }, | ||
// }); | ||
// const { deletePad } = await reqsParams; | ||
// expect(deletePad?.get('padID')).toEqual(item.extra.etherpad.padID); | ||
// }); | ||
|
||
// it('copies pad when item is copied', async () => { | ||
// const parent = await testUtils.saveItemAndMembership({ | ||
// member, | ||
// item: { name: 'test parent' }, | ||
// }); | ||
|
||
// const reqsParams = setUpApi({ | ||
// createGroupIfNotExistsFor: [ | ||
// StatusCodes.OK, | ||
// { code: 0, message: 'ok', data: { groupID: MOCK_GROUP_ID } }, | ||
// ], | ||
// copyPad: [StatusCodes.OK, { code: 0, message: 'ok', data: null }], | ||
// }); | ||
|
||
// await app.inject({ | ||
// method: 'POST', | ||
// url: 'items/copy', | ||
// query: { | ||
// id: [item.id], | ||
// }, | ||
// payload: { | ||
// parentId: parent.item.id, | ||
// }, | ||
// }); | ||
|
||
// // wait until the copy is in the db | ||
// await waitForExpect(async () => { | ||
// expect(await testUtils.rawItemRepository.count()).toEqual(3); | ||
// }); | ||
|
||
// const copy = (await testUtils.rawItemRepository.findOneBy({ | ||
// id: And(Not(item.id), Not(parent.item.id)), | ||
// })) as Item<typeof ItemType.ETHERPAD>; | ||
// expect(copy).not.toBeNull(); | ||
|
||
// const { createGroupIfNotExistsFor, copyPad } = await reqsParams; | ||
// expect(copyPad?.get('destinationID')).toEqual( | ||
// `${copy.extra.etherpad.groupID}$${createGroupIfNotExistsFor?.get('groupMapper')}`, | ||
// ); | ||
// expect(copyPad?.get('sourceID')).toEqual(item.extra.etherpad.padID); | ||
// // verify that the handler mutated the item on its extra (should have created a newly copied pad) | ||
// expect(item.extra).not.toEqual(copy.extra); | ||
// }); | ||
|
||
// it('throws if pad ID is not defined on copy', async () => { | ||
// const { item: parent } = await testUtils.saveItemAndMembership({ member }); | ||
// const { item: bogusItem } = await testUtils.saveItemAndMembership({ member }); | ||
|
||
// setUpApi({ | ||
// createGroupIfNotExistsFor: [ | ||
// StatusCodes.OK, | ||
// { code: 0, message: 'ok', data: { groupID: MOCK_GROUP_ID } }, | ||
// ], | ||
// copyPad: [StatusCodes.OK, { code: 0, message: 'ok', data: null }], | ||
// }); | ||
|
||
// const res = await app.inject({ | ||
// method: 'POST', | ||
// url: 'items/copy', | ||
// query: { | ||
// id: [bogusItem.id], | ||
// }, | ||
// payload: { | ||
// parentId: parent.id, | ||
// }, | ||
// }); | ||
|
||
// expect(res.statusCode).not.toEqual(StatusCodes.OK); | ||
// }); | ||
// }); | ||
// }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests should not be commented out. Please uncomment the test suite.
Commenting out the entire test suite for the Etherpad service API is not advisable as it disables all the test cases. This can lead to potential undetected regressions or issues in the functionality when making future changes.
Tests are crucial for maintaining code quality and preventing bugs. They should be kept active and updated as the code evolves. Consider uncommenting these tests and ensuring they pass. If there are specific tests that are no longer relevant, consider removing them entirely rather than commenting them out.
Tools
Gitleaks
32-32: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will remain this way. We are keeping the tests commented out because Etherpad and H5P are now taken out but may need to be re-enabled later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymijsters I understand your decision to keep the tests commented out due to the removal of Etherpad and H5P functionality, with the possibility of re-enabling them later. Thank you for providing that context.
While this approach can work, here are some suggestions that might help manage this situation more effectively:
-
Consider using feature flags or conditional compilation instead of commenting out large blocks of code. This can make it easier to toggle functionality on and off without cluttering the codebase.
-
If you decide to keep the tests commented out, it might be helpful to add a prominent comment at the top of the file explaining why the tests are commented out and under what conditions they should be re-enabled. This will help other developers (or yourself in the future) understand the current state of the code.
-
You could move the commented-out tests to a separate file (e.g.,
etherpad.test.ts.bak
) to keep your active test files clean while still preserving the test code for future use. -
If possible, consider keeping a minimal set of tests uncommented to ensure basic functionality is still intact, even if the full feature set is currently disabled.
These are just suggestions, and you should choose the approach that best fits your team's workflow and project requirements. Let me know if you have any questions or if there's anything else I can help with!
(´。• ω •。`) ♡
Learnings added
Learnt from: ymijsters
PR: lnco-epfl/graasp#12
File: src/services/item/plugins/etherpad/test/index.test.ts:1-768
Timestamp: 2024-09-12T08:24:56.678Z
Learning: The test file `src/services/item/plugins/etherpad/test/index.test.ts` has been completely commented out as part of the removal of Etherpad functionality.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Learnings used
Learnt from: ymijsters
PR: lnco-epfl/graasp#12
File: src/services/item/plugins/etherpad/index.ts:1-2
Timestamp: 2024-09-12T08:09:31.557Z
Learning: The test file `src/services/item/plugins/etherpad/test/index.test.ts` has been completely commented out as part of the removal of Etherpad functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed it after all. Could you recheck?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ymijsters Thank you for following up. I've rechecked the repository, and I can confirm that the Etherpad plugin directory, including the test file src/services/item/plugins/etherpad/test/index.test.ts
, has been completely removed. This is an excellent resolution to our previous discussion about commented-out tests.
By removing the entire Etherpad plugin, you've effectively cleaned up the codebase and eliminated the concern about inactive tests. This approach is much cleaner than keeping commented-out code and aligns well with best practices for code maintenance.
Is there anything else you'd like me to review or any other changes you've made that you want me to check?
(⌒‿⌒)
Analysis chain
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the file still exists
fd -t f "index.test.ts" src/services/item/plugins/etherpad/test
Length of output: 190
Script:
#!/bin/bash
# Check if the etherpad directory still exists
fd -t d "etherpad" src/services/item/plugins
Length of output: 44
Learnings added
Learnt from: ymijsters
PR: lnco-epfl/graasp#12
File: src/services/item/plugins/etherpad/test/index.test.ts:0-0
Timestamp: 2024-09-12T13:00:46.829Z
Learning: The entire Etherpad plugin directory, including tests, has been removed from the project structure at src/services/item/plugins/etherpad/.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Learnings removed
Learnt from: ymijsters
PR: lnco-epfl/graasp#12
File: src/services/item/plugins/etherpad/test/index.test.ts:1-768
Timestamp: 2024-09-12T08:24:56.917Z
Learning: The test file `src/services/item/plugins/etherpad/test/index.test.ts` has been completely commented out as part of the removal of Etherpad functionality.
Learnt from: ymijsters
PR: lnco-epfl/graasp#12
File: src/services/item/plugins/etherpad/index.ts:1-2
Timestamp: 2024-09-12T08:09:31.557Z
Learning: The test file `src/services/item/plugins/etherpad/test/index.test.ts` has been completely commented out as part of the removal of Etherpad functionality.
Learnings used
Learnt from: ymijsters
PR: lnco-epfl/graasp#12
File: src/services/item/plugins/etherpad/test/index.test.ts:1-768
Timestamp: 2024-09-12T08:24:56.917Z
Learning: The test file `src/services/item/plugins/etherpad/test/index.test.ts` has been completely commented out as part of the removal of Etherpad functionality.
Learnt from: ymijsters
PR: lnco-epfl/graasp#12
File: src/services/item/plugins/etherpad/index.ts:1-2
Timestamp: 2024-09-12T08:09:31.557Z
Learning: The test file `src/services/item/plugins/etherpad/test/index.test.ts` has been completely commented out as part of the removal of Etherpad functionality.
* 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
* 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
* 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
* 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
* 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
* 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
Removed Etherpad, H5P and Meilisearch from the back end (including all relevant routes and repositories). Decided to retain Redis as it is quite omnipresent and might have some useful features going forward. This version has been deployed from this branch to the dev environment and supports the online LNCO version of Graasp.
Front-end features related to Etherpad and H5P still need to be removed, issues have been created for this.
Note that as this was the first commit to the fork, there are quite some file changes forced by prettier. There were also some commit/push issues in husky that needed to be resolved.
Summary by CodeRabbit
Bug Fixes
SearchService
and its interactions with theMeiliSearchWrapper
.Documentation
Chores
package.json
, enhancing tooling for commit message quality.