fix: EduID for QA environment, transactional emails based on DB templates, misc fixes #4206
Conversation
Current Aviator status
This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
WalkthroughWalkthroughThe recent changes involve updates to multiple GitHub Actions workflow files, primarily removing the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
klicker-uzh
|
||||||||||||||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
v3
|
| Run status |
|
| Run duration | 08m 05s |
| Commit |
|
| Committer | Roland Schläfli |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
1
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
37
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
cypress/e2e/K-group-activity-workflow.cy.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| Create and solve a group activity > create a group activity and edit it |
Screenshots
|
|
cypress/e2e/G-microlearning-workflow.cy.ts • 1 flaky test
| Test | Artifacts | |
|---|---|---|
| Different microlearning workflows > converts a seeded past microlearning into a practice quiz |
Test Replay
Screenshots
|
|
rschlaefli
changed the title
rschlaefli
changed the title
rschlaefli
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (30)
- .github/workflows/v3_analytics-prod.yml (1 hunks)
- .github/workflows/v3_analytics-qa.yml (1 hunks)
- .github/workflows/v3_auth-prod.yml (1 hunks)
- .github/workflows/v3_auth-qa.yml (1 hunks)
- .github/workflows/v3_backend-docker.yml (1 hunks)
- .github/workflows/v3_frontend-control-docker-prod.yml (1 hunks)
- .github/workflows/v3_frontend-control-docker-qa.yml (1 hunks)
- .github/workflows/v3_frontend-manage-docker-prod.yml (1 hunks)
- .github/workflows/v3_frontend-manage-docker-qa.yml (1 hunks)
- .github/workflows/v3_frontend-pwa-docker-prod.yml (1 hunks)
- .github/workflows/v3_frontend-pwa-docker-qa.yml (1 hunks)
- .github/workflows/v3_lti-prod.yml (1 hunks)
- .github/workflows/v3_lti-qa.yml (1 hunks)
- apps/auth/src/pages/api/auth/[...nextauth].ts (1 hunks)
- apps/backend-docker/package.json (2 hunks)
- apps/backend-docker/tsup.config.ts (1 hunks)
- apps/frontend-pwa/.env.qa (1 hunks)
- apps/frontend-pwa/src/lib/getParticipantToken.ts (1 hunks)
- apps/frontend-pwa/src/pages/createAccount.tsx (2 hunks)
- apps/lti/src/index.ts (1 hunks)
- deploy/charts/klicker-uzh-v2/templates/cm-lti.yaml (1 hunks)
- deploy/charts/klicker-uzh-v2/templates/deployment-app.yaml (1 hunks)
- deploy/charts/klicker-uzh-v2/values.yaml (1 hunks)
- deploy/env-qa-v3/helmfile.yaml (2 hunks)
- packages/graphql/src/services/accounts.ts (3 hunks)
- packages/graphql/src/services/email.ts (2 hunks)
- packages/prisma/src/prisma/migrations/20240823205930_email_templates/migration.sql (1 hunks)
- packages/prisma/src/prisma/schema/other.prisma (1 hunks)
- packages/transactional/package.json (1 hunks)
- packages/transactional/src/scripts/deploy.ts (1 hunks)
Files skipped from review due to trivial changes (5)
- .github/workflows/v3_frontend-control-docker-qa.yml
- .github/workflows/v3_lti-qa.yml
- apps/auth/src/pages/api/auth/[...nextauth].ts
- apps/backend-docker/package.json
- apps/backend-docker/tsup.config.ts
Additional comments not posted (27)
packages/prisma/src/prisma/schema/other.prisma (1)
7-14: LGTM! TheEmailTemplatemodel is well-structured.The addition of the
EmailTemplatemodel with fields forname,html,createdAt, andupdatedAtis appropriate for managing email templates. The use of@id,@default(now()), and@updatedAtis correct and aligns with best practices.packages/prisma/src/prisma/migrations/20240823205930_email_templates/migration.sql (2)
4-5: Verify the impact of dropping columns fromParticipant.Dropping
magicLinkExpiresAtandmagicLinkTokenwill result in data loss. Ensure that this change is intentional and that any necessary data has been backed up or migrated.
12-20: LGTM! TheEmailTemplatetable creation is consistent with the schema.The creation of the
EmailTemplatetable with fieldsname,html,createdAt, andupdatedAtis consistent with the schema changes inother.prisma.apps/frontend-pwa/.env.qa (1)
8-8: Verify the new SSR API endpoint URL.The
NEXT_PUBLIC_API_URL_SSRhas been updated to a new backend URL. Ensure that this URL is correct and that the backend service is operational.packages/transactional/package.json (2)
14-17: Scripts for updating templates look good.The new scripts for updating templates in different environments are well-formed and use the
dopplercommand with the correct configurations.
20-27: Dependency updates are appropriate.The addition of
@klicker-uzh/prismaandtsx, along with the updates to@types/nodeand@types/react, enhance the project's functionality and maintainability.deploy/charts/klicker-uzh-v2/templates/cm-lti.yaml (1)
23-24: New LTI configuration parameters are correctly added.The parameters
LTI_DEV_MODEandLTI_AAS_MODEare properly sourced from the Helm chart values, enhancing configurability..github/workflows/v3_lti-prod.yml (1)
3-5: Removal ofpathsfilter is appropriate.The removal of the
pathsfilter aligns with the objective to broaden the conditions under which the workflow is executed, allowing for more inclusive builds..github/workflows/v3_auth-prod.yml (1)
3-5: LGTM! Broadened workflow trigger conditions.The removal of the
pathsfilter from thepushevent trigger allows the workflow to trigger on any push event with a matching tag, enhancing automation and ensuring comprehensive builds..github/workflows/v3_frontend-pwa-docker-prod.yml (1)
3-5: LGTM! Broadened workflow trigger conditions.The removal of the
pathsfilter from thepushevent trigger allows the workflow to trigger on any push event with a matching tag, enhancing deployment flexibility and ensuring comprehensive builds..github/workflows/v3_frontend-manage-docker-prod.yml (1)
3-5: LGTM! Broadened workflow trigger conditions.The removal of the
pathsfilter from thepushevent trigger allows the workflow to trigger on any push event with a matching tag, improving the deployment process and ensuring comprehensive builds..github/workflows/v3_frontend-control-docker-prod.yml (1)
4-5: Approved: Broadened workflow trigger conditions.The removal of the
pathsfilter from thepushevent trigger allows the workflow to trigger on any push event matching the specified tag pattern. This change aligns with the PR objectives to enhance automation..github/workflows/v3_analytics-prod.yml (1)
4-5: Approved: Broadened workflow trigger conditions.The removal of the
pathsfilter from thepushevent trigger allows the workflow to trigger on any push event matching the specified tag pattern. This change aligns with the PR objectives to enhance automation..github/workflows/v3_analytics-qa.yml (1)
Line range hint
4-8: Consider the implications of removing thepathsfilter.The removal of the
pathsfilter under thepushevent trigger will cause the workflow to run on any changes to the specified branches. While this broadens the scope for testing, it may lead to unnecessary workflow executions for unrelated changes.Ensure that this change aligns with your intended workflow execution strategy.
.github/workflows/v3_backend-docker.yml (1)
Line range hint
4-9: Evaluate the impact of removing thepathsfilter.The removal of the
pathsfilter under thepushevent trigger expands the workflow's execution scope to any changes on the specified branches. This may result in additional workflow runs for changes that do not affect the backend-docker.Verify that this change is intentional and aligns with your workflow execution goals.
.github/workflows/v3_auth-qa.yml (1)
Line range hint
4-8: Assess the consequences of removing thepathsfilter.By removing the
pathsfilter under thepushevent trigger, the workflow will now execute on any changes to the specified branches. This could lead to more comprehensive testing but may also result in unnecessary workflow executions.Ensure that this change is consistent with your desired workflow execution strategy.
.github/workflows/v3_frontend-pwa-docker-qa.yml (1)
Line range hint
5-11: Verify the impact of removing path filters.The removal of path filters under the
pushevent trigger will cause the workflow to trigger on any push to the specified branches. Ensure this aligns with the intended CI/CD process to avoid unnecessary workflow runs..github/workflows/v3_frontend-manage-docker-qa.yml (1)
Line range hint
5-11: Verify the impact of removing path filters.The removal of path filters under the
pushevent trigger will cause the workflow to trigger on any push to the specified branches. Ensure this aligns with the intended CI/CD process to avoid unnecessary workflow runs.packages/graphql/src/services/email.ts (1)
49-68: LGTM! But verify the function usage in the codebase.The changes to
hydrateTemplateimprove flexibility and scalability. Ensure all function calls tohydrateTemplatematch the new signature with thectxparameter.Run the following script to verify the function usage:
Verification successful
Function Usage Verified: All calls to
hydrateTemplatematch the new signature.The function
hydrateTemplateis used correctly with thectxparameter in the codebase. No further action is required.
packages/graphql/src/services/accounts.ts: Both occurrences ofhydrateTemplateinclude thectxparameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `hydrateTemplate` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type js -A 5 $'hydrateTemplate'Length of output: 35
Script:
#!/bin/bash # Description: Verify all function calls to `hydrateTemplate` match the new signature. # Search for the function usage in the entire codebase, including TypeScript files. rg --type-add 'ts:*.{ts,tsx}' --type ts --type js -A 5 'hydrateTemplate'Length of output: 1285
apps/lti/src/index.ts (1)
14-15: LGTM! Verify the environment variables.The changes to use
process.env.LTI_DEV_MODEandprocess.env.LTI_AAS_MODEprovide more granular control over the environment settings.Ensure that these environment variables are correctly set in all relevant environments.
deploy/env-qa-v3/helmfile.yaml (1)
29-31: LGTM! Verify the environment variables.The use of
requiredEnvforEDUID_CLIENT_SECRETandAPP_VERSIONenhances security and ensures deployment consistency.Ensure that these environment variables are correctly set in all relevant environments.
Also applies to: 96-98
apps/frontend-pwa/src/pages/createAccount.tsx (1)
65-66: Verify the redirection path change.The redirection path has been changed from
/loginto/editProfile. Ensure this change aligns with the intended user flow and does not introduce any unintended behavior.deploy/charts/klicker-uzh-v2/values.yaml (1)
279-281: Verify new configuration options.The
devModeandltiaasoptions have been added. Ensure these configurations are documented and align with the application's environment management strategy.deploy/charts/klicker-uzh-v2/templates/deployment-app.yaml (1)
433-433: Verify dynamic image tag configuration.The image tag for the
lticontainer now uses a dynamic reference. Ensure that the image tag is correctly configured in the Helm chart values to prevent deployment issues.packages/graphql/src/services/accounts.ts (3)
596-615: LGTM! Verify logging practices.The added logging statements provide useful debugging information.
Ensure that sensitive information is not logged and that logs are appropriately managed.
Run the following script to verify the logging practices:
239-245: LGTM! Verify rate limiting logic.The addition of the
ctxparameter for email template hydration is a good improvement for context handling.Ensure that the rate limiting logic is functioning as expected and is not prone to bypass.
Run the following script to verify the rate limiting logic:
Verification successful
Rate Limiting Logic Verified
The rate limiting logic in the
sendMagicLinkfunction is correctly implemented. It ensures that users cannot exceed 5 requests per hour, effectively preventing abuse.
- The
rateLimitStoretracks request counts and timestamps.- The logic correctly resets counts after the time window.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the rate limiting logic in `sendMagicLink`. # Test: Search for rate limiting logic. Expect: Correct implementation without bypass. rg --type javascript -A 5 'rateLimitStore'Length of output: 77
Script:
#!/bin/bash # Description: Verify the rate limiting logic in `sendMagicLink`. # Test: Search for rate limiting logic. Expect: Correct implementation without bypass. rg --type ts -A 5 'rateLimitStore'Length of output: 1901
540-554: LGTM! Verify activation link generation logic.The addition of the
ctxparameter for email template hydration and the improved construction of theactivationLinkenhance context handling and readability.Ensure that the activation link generation logic is secure and functioning as expected.
Run the following script to verify the activation link generation logic:
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/transactional/src/scripts/deploy.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/transactional/src/scripts/deploy.ts
klicker-uzh
|
||||||||||||||||||||||||||||||||||||||||
| Project |
klicker-uzh
|
| Branch Review |
v3-lti
|
| Run status |
|
| Run duration | 08m 24s |
| Commit |
|
| Committer | Roland Schläfli |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
Failures
|
1
|
|
Flaky
|
1
|
|
Pending
|
0
|
|
Skipped
|
0
|
|
Passing
|
37
|
| View all changes introduced in this branch ↗︎ | |
Tests for review
cypress/e2e/F-live-quiz-workflow.cy.ts • 1 failed test
| Test | Artifacts | |
|---|---|---|
| Different live-quiz workflows > creates a live quiz without questions and tests the feedback mechanisms and deletes the completed live session |
Test Replay
Screenshots
|
|
cypress/e2e/K-group-activity-workflow.cy.ts • 1 flaky test
| Test | Artifacts | |
|---|---|---|
| Create and solve a group activity > create a group activity and edit it |
Screenshots
|
|
|
No description provided.