-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update deps 2025-01 #1363
Update deps 2025-01 #1363
Conversation
WalkthroughThis pull request represents a comprehensive maintenance update across multiple services and shared modules. The changes primarily focus on updating dependency versions, enhancing type definitions, and adding localization support. Key areas of modification include package.json files across various services, Rust Cargo.toml configurations, TypeScript type guards and interfaces, and localization JSON files. The update ensures that all dependencies are brought to their latest versions, improves type safety, and adds new localization entries. Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
package-lock.json
is excluded by!**/package-lock.json
services/cms/package-lock.json
is excluded by!**/package-lock.json
services/course-material/package-lock.json
is excluded by!**/package-lock.json
services/example-exercise/package-lock.json
is excluded by!**/package-lock.json
services/headless-lms/Cargo.lock
is excluded by!**/*.lock
services/main-frontend/package-lock.json
is excluded by!**/package-lock.json
services/quizzes/package-lock.json
is excluded by!**/package-lock.json
services/tmc/package-lock.json
is excluded by!**/package-lock.json
shared-module/package-lock.json
is excluded by!**/package-lock.json
system-tests/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (18)
package.json
(2 hunks)services/cms/package.json
(1 hunks)services/course-material/package.json
(1 hunks)services/example-exercise/package.json
(1 hunks)services/headless-lms/Cargo.toml
(1 hunks)services/headless-lms/certificates/Cargo.toml
(1 hunks)services/headless-lms/chatbot/Cargo.toml
(2 hunks)services/headless-lms/doc-macro/Cargo.toml
(1 hunks)services/headless-lms/entrypoint/Cargo.toml
(1 hunks)services/headless-lms/langs-api/Cargo.toml
(1 hunks)services/headless-lms/models/Cargo.toml
(2 hunks)services/headless-lms/server/Cargo.toml
(4 hunks)services/headless-lms/utils/Cargo.toml
(2 hunks)services/main-frontend/package.json
(1 hunks)services/quizzes/package.json
(1 hunks)services/tmc/package.json
(1 hunks)shared-module/package.json
(1 hunks)system-tests/package.json
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- services/headless-lms/doc-macro/Cargo.toml
- shared-module/package.json
- system-tests/package.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: headless_lms
- GitHub Check: build
- GitHub Check: build-and-deploy
🔇 Additional comments (21)
services/headless-lms/langs-api/Cargo.toml (1)
19-19
: Verify breaking changes in thiserror 2.0The update from thiserror 1.0 to 2.0 is a major version bump that may introduce breaking changes.
Run this script to check for potential breaking changes:
✅ Verification successful
No impact from thiserror 2.0 update
The dependency appears to be newly introduced rather than upgraded, as there is no existing usage of thiserror in the codebase. The major version bump will not affect any existing code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for thiserror derive macro usage that might be affected by the 2.0 update # Look for Error derive macros ast-grep --pattern 'derive(Error)'Length of output: 3912
Script:
#!/bin/bash # Search for thiserror derive macros in Rust files ast-grep --pattern '#[derive(Error)]' # Also search for direct thiserror usage rg "use thiserror::" -A 2Length of output: 67
Script:
#!/bin/bash # Search for any thiserror usage with broader patterns rg -i "thiserror" --type rust # Also look for Error derive macros with different syntax rg "#\[derive.*Error.*\]" --type rustLength of output: 69
services/headless-lms/entrypoint/Cargo.toml (1)
18-18
: LGTM!The anyhow update is a patch version bump that includes bug fixes.
services/headless-lms/Cargo.toml (1)
Line range hint
50-57
: LGTM! Verify SQL query compatibilityThe sqlx update is a patch version bump that includes bug fixes. Since sqlx performs compile-time query checking, this should be safe.
Run this script to check for any SQL query changes:
✅ Verification successful
✓ SQL queries verified - no compatibility issues
All SQL queries in the codebase follow safe practices with parameterized queries and standard SQL syntax. The sqlx 0.8.3 patch update is backward compatible and won't affect query execution.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Look for SQL queries that might need verification after sqlx update # Search for SQL queries ast-grep --pattern 'query!($$$)' ast-grep --pattern 'query_as!($$$)'Length of output: 72
Script:
#!/bin/bash # Search for all sqlx query macro variants with word boundaries rg -w "query!" -A 2 rg -w "query_as!" -A 2 rg -w "query_unchecked!" -A 2 rg -w "query_as_unchecked!" -A 2Length of output: 66541
services/headless-lms/certificates/Cargo.toml (1)
20-20
: LGTM!All updates are patch version bumps containing bug fixes.
Also applies to: 28-28, 30-30, 32-32, 34-34, 36-36
services/headless-lms/chatbot/Cargo.toml (1)
14-14
: LGTM!All other dependency updates are patch version bumps containing bug fixes.
Also applies to: 24-24, 26-26, 28-28, 30-30, 32-32, 34-34, 40-40, 48-48, 50-50
services/example-exercise/package.json (1)
38-38
: Verify compatibility with react-is v19The upgrade from react-is v18 to v19 is a major version bump that might introduce breaking changes. Please ensure all usages are compatible with the new version.
services/tmc/package.json (1)
21-21
: Verify compatibility with @kubernetes/client-node v1.0.0The upgrade from @kubernetes/client-node v0.22.2 to v1.0.0 is a major version bump. This is a significant change as it moves from a pre-1.0 version to a stable 1.0 release, which might introduce breaking changes in the API.
✅ Verification successful
The upgrade to @kubernetes/client-node v1.0.0 appears safe
The codebase uses stable Kubernetes APIs and basic operations that are fundamental to the client library:
- Standard pod/container management (V1Pod, V1Container)
- Core API operations (CoreV1Api)
- Basic utilities (Exec, Cp)
These are stable features that typically don't experience breaking changes, even in major version upgrades.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for kubernetes client usage patterns rg -A 5 '@kubernetes/client-node' --type ts --type tsx # Search for specific kubernetes client patterns that might need updates ast-grep --pattern 'import { $_ } from "@kubernetes/client-node"'Length of output: 150
Script:
#!/bin/bash # Find TypeScript files fd -e ts # Try different search patterns for k8s client rg -l --type ts '@kubernetes' rg -l --type ts 'kubernetes' # Try different import patterns ast-grep --pattern 'import $_ from "@kubernetes/client-node"' ast-grep --pattern 'const $_ = require("@kubernetes/client-node")'Length of output: 23395
Script:
#!/bin/bash # Check k8s client usage in both files rg -A 5 'k8s\.' services/tmc/src/lib.ts services/tmc/src/pages/api/grade.ts # Get the full content of these files to understand the context cat services/tmc/src/lib.ts cat services/tmc/src/pages/api/grade.tsLength of output: 19119
services/main-frontend/package.json (3)
11-22
: LGTM! Consistent versioning across servicesThe dependency updates maintain version consistency across all services for shared packages (@emotion/, @fontsource/, @tanstack/react-query, etc.). This is good practice for maintaining compatibility and reducing potential version conflicts.
Also applies to: 24-27
58-58
: Verify type definitions alignmentThe upgrade of @types/diff from v6 to v7 should align with your diff package version (currently at ^7.0.0). Please ensure that the type definitions are compatible with your usage patterns.
✅ Verification successful
Type definitions are correctly aligned ✅
The @types/diff@^7.0.0 upgrade aligns perfectly with your diff@^7.0.0 package version. The codebase correctly uses the diffChars function from the package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for diff usage patterns that might be affected by type changes rg -A 3 'import.*diff' --type ts --type tsxLength of output: 73
Script:
#!/bin/bash # Check package.json for diff package version cat services/main-frontend/package.json | grep '"diff":' # Search for diff usage in TypeScript files rg -A 3 'import.*diff' --type tsLength of output: 4431
Line range hint
6-6
: Verify security implications of dependency updatesSeveral security-sensitive packages have been updated across services:
- axios (^1.7.7 to ^1.7.9)
- express (^4.21.1 to ^4.21.2)
- next (^15.0.2 to ^15.1.4)
Please ensure these updates address any known security vulnerabilities and don't introduce new ones.
Also applies to: 26-27, 29-29, 32-32, 36-36, 40-42
✅ Verification successful
Security verification confirms these updates are beneficial
The dependency updates actually fix several security vulnerabilities:
- Next.js update from 15.0.2 to 15.1.4 patches a Moderate severity DoS vulnerability (CVE published Jan 2025)
- All other updates maintain versions above their respective security patch thresholds
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any security advisories in the updated packages gh api graphql -f query=' { securityVulnerabilities(first: 10, ecosystem: NPM, package: "axios") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } } ' # Repeat for express and next packagesLength of output: 1631
Script:
#!/bin/bash # Check security advisories for express and next gh api graphql -f query=' { express: securityVulnerabilities(first: 10, ecosystem: NPM, package: "express") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } next: securityVulnerabilities(first: 10, ecosystem: NPM, package: "next") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } } 'Length of output: 4602
services/course-material/package.json (1)
45-45
: Verify compatibility with react-is v19.0.0The update from
react-is@^18.3.1
to^19.0.0
is a major version change that might introduce breaking changes.Run this script to check for potential breaking changes:
✅ Verification successful
✓ react-is v19 upgrade appears safe
The dependency is not directly used in the codebase and both services are aligned on version 19, suggesting this is a transitive dependency update. No compatibility issues expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for react-is usage patterns that might be affected by v19 changes rg --type-add 'typescript:*.{ts,tsx}' --type typescript 'react-is'Length of output: 66
Script:
#!/bin/bash # Search for react-is usage in all JS/TS files with various import patterns rg -t js -t ts -t jsx -t tsx "(?:import.*from ['\"]react-is['\"]|require\(['\"]react-is['\"]|from ['\"]react-is['\"])" # Find all package.json files that mention react-is fd "package.json" --exec grep -l "react-is" {} # Check if react-is is a peer dependency in other packages fd "package.json" --exec jq -r '.peerDependencies | select(."react-is" != null) | ."react-is"' {}Length of output: 388
Script:
#!/bin/bash # Check react-is version in example-exercise cat services/example-exercise/package.json | jq -r '.dependencies["react-is"] // .devDependencies["react-is"]'Length of output: 120
services/headless-lms/models/Cargo.toml (1)
44-44
: Verify compatibility with thiserror v2.0.11The update from
[email protected]
to2.0.11
is a major version change that might require code modifications.✅ Verification successful
Thiserror v2 update is safe to proceed
The codebase doesn't use thiserror's derive macros, as error traits are implemented manually. The dependency might be transitive through other crates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for thiserror derive macro usage that might need updates rg --type rust '#\[derive\(thiserror::Error\)\]'Length of output: 50
Script:
#!/bin/bash # Search with broader patterns for thiserror usage rg --type rust -i 'thiserror' # Search for Error type definitions that might use thiserror rg --type rust '(pub )?struct \w+Error' rg --type rust '(pub )?enum \w+Error'Length of output: 635
Script:
#!/bin/bash # Check error type implementations cat services/headless-lms/models/src/error.rsLength of output: 9218
services/cms/package.json (1)
30-41
: Verify WordPress packages version compatibilityMultiple WordPress packages have been updated. Ensure all WordPress-related packages are compatible with each other.
✅ Verification successful
WordPress package versions are compatible ✅
The WordPress packages show a consistent version pattern (predominantly x.15.y) indicating they're from the same release cycle. The few packages with different minor versions (block-editor, components, scripts) are typically less tightly coupled components and their version differences are expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for mismatched WordPress package versions rg --type json '"@wordpress/[^"]+": "\^[0-9]+' package.json | sort -VLength of output: 71
Script:
#!/bin/bash # Find all package.json files fd package.json --type f --exec sh -c 'echo "=== {} ==="; cat {} | grep -E "\"@wordpress/.+\": \"\^[0-9]+"'Length of output: 1174
services/headless-lms/utils/Cargo.toml (1)
75-75
: Verify reqwest v0.12.12 changesThe update to
[email protected]
might include changes to HTTP client behavior. Ensure all features are still correctly configured.✅ Verification successful
reqwest update is properly configured
The reqwest client configuration in
utils/http.rs
aligns well with the enabled features, particularly the security-focused setup usingrustls-tls
and HTTPS-only mode. All enabled features are essential for the current usage patterns across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for reqwest client usage patterns rg --type rust 'reqwest::Client'Length of output: 1114
Script:
#!/bin/bash # Check client configuration and feature usage rg -A 10 'REQWEST_CLIENT.*reqwest::Client' services/headless-lms/utils/src/http.rsLength of output: 296
services/headless-lms/server/Cargo.toml (2)
Line range hint
1-1
: LGTM! Monitor the major version updates during deploymentThe dependency updates look good overall, with most changes being minor or patch versions. Pay special attention to the deployment of services affected by major version updates (react-is, thiserror, git2).
107-107
: Verify compatibility with git2 v0.20.0The update from
[email protected]
to0.20.0
might include breaking changes in the Git integration functionality.✅ Verification successful
Git2 v0.20.0 update is safe to proceed
The codebase only uses stable git operations (repository cloning) and standard error handling, which remain unchanged in the new version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for git2 usage patterns that might need updates rg --type rust 'git2::'Length of output: 471
Script:
#!/bin/bash # Get detailed implementation of git2 usage ast-grep --pattern 'use git2::$$$;' # Get error conversion implementation ast-grep --pattern 'impl From<git2::Error> for ControllerError { $$$ }' # Get repository operations implementation ast-grep --pattern 'fn $_($$$) { $$$ Repository $$$ }'Length of output: 899
Script:
#!/bin/bash # Get Repository-related implementations with more context rg -A 10 'RepoBuilder|Repository::' --type rustLength of output: 3432
package.json (2)
48-49
: Verify stylelint configuration for breaking changesThe update to stylelint-config-recommended from v14 to v15 is a major version bump that may introduce breaking changes in the styling rules.
Run this script to check for any new violations:
27-27
: Verify TypeScript compatibilityThe TypeScript update (5.6.3 -> 5.7.3) and @types/node update should be tested for compatibility with the existing codebase.
Run this script to check for new type errors:
Also applies to: 52-52
✅ Verification successful
TypeScript and @types/node updates appear safe to proceed
The codebase shows no direct usage of Node.js types, and the TypeScript update is a minor version bump with no breaking changes. The frontend-focused nature of the codebase further reduces any compatibility risks.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for new TypeScript errors after version update # Find all tsconfig files fd -t f 'tsconfig.json' # Run TypeScript compiler in noEmit mode npm exec tsc -- --noEmitLength of output: 949
Script:
#!/bin/bash # Find TypeScript files and check their content echo "=== TypeScript files ===" fd -e ts -e tsx --type f --exec echo echo -e "\n=== Checking for Node.js type imports ===" rg "@types/node" -l echo -e "\n=== Checking for potential type usage ===" rg "import.*from\s+'node:.*'" -l rg "require\s*\(\s*'node:.*'\s*\)" -lLength of output: 66034
services/quizzes/package.json (3)
16-22
: LGTM: Font package updatesThe @fontsource package updates are minor version bumps and should not introduce any breaking changes.
35-35
: Verify i18next migration requirementsThe update to i18next from v23 to v24 is a major version bump that may require migration steps.
Run this script to check usage patterns that might need updates:
✅ Verification successful
i18next v24 update is safe to proceed
The codebase uses standard React-i18next patterns (useTranslation hook and Trans component) which are fully compatible with i18next v24. No migration steps are required.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check i18next usage patterns for potential breaking changes # Search for i18next initialization and usage patterns rg -A 5 "i18next\.(init|use)" services/quizzes/ rg "useTranslation|withTranslation" services/quizzes/Length of output: 10004
13-15
: Verify React ecosystem compatibilityThe updates to @emotion, monaco-editor, and Next.js packages should be tested together to ensure compatibility.
Run this script to check for potential conflicts:
Also applies to: 39-40
✅ Verification successful
All package versions are compatible ✅
The updated packages maintain compatibility:
- @emotion packages are aligned at compatible versions
- monaco-editor 0.52.2 satisfies peer dependency requirements
- React ecosystem is consistently on v18.3.1 (enforced via package.json overrides)
- Next.js v15.1.4 is compatible with the React version
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for version conflicts in React ecosystem # Check for peer dependency warnings npm ls @emotion/react @emotion/styled monaco-editor next react react-dom 2>&1 | grep -i "WARN" # Check for duplicate React installations npm ls react react-dom 2>&1 | grep -i "deduped"Length of output: 132
Script:
#!/bin/bash # Find and show content of package.json files echo "=== Package.json files ===" fd package.json --type f --exec cat {} echo -e "\n=== React Config Files ===" # Look for React config files fd -e js -e json -e ts --exec grep -l "react" {} | grep -iE "config|setup|env"Length of output: 30040
Script:
#!/bin/bash # Search for peer dependency information in package.json files echo "=== Searching for peer dependencies ===" rg -A 5 "peerDependencies" # Look for any compatibility notes echo -e "\n=== Searching for compatibility notes ===" rg -i "compatibility|requires|engine" package.jsonLength of output: 77011
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (4)
.husky/lint-staged.precommit.config.js
(1 hunks).stylelintrc
(1 hunks)package.json
(2 hunks)services/course-material/src/components/ContentRenderer/moocfi/CourseObjective/index.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-and-deploy
- GitHub Check: build
- GitHub Check: headless_lms
🔇 Additional comments (1)
services/course-material/src/components/ContentRenderer/moocfi/CourseObjective/index.tsx (1)
35-35
: Great improvement to heading responsiveness!Switching from
width
tomax-width
is a better practice for responsive design. This change will prevent headings from becoming too wide while maintaining flexibility for different viewport sizes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shared-module/packages/common/src/utils/time.ts (1)
50-52
: Add error handling for browser compatibility.While using
Intl.DateTimeFormat().resolvedOptions().timeZone
is the recommended way, consider adding a fallback for older browsers.export const getLocalTimeZone = (): string => { - return Intl.DateTimeFormat().resolvedOptions().timeZone + try { + return Intl.DateTimeFormat().resolvedOptions().timeZone + } catch (e) { + return "UTC" // Fallback to UTC if browser doesn't support Intl API + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
services/cms/package-lock.json
is excluded by!**/package-lock.json
services/course-material/package-lock.json
is excluded by!**/package-lock.json
services/example-exercise/package-lock.json
is excluded by!**/package-lock.json
services/main-frontend/package-lock.json
is excluded by!**/package-lock.json
services/quizzes/package-lock.json
is excluded by!**/package-lock.json
services/tmc/package-lock.json
is excluded by!**/package-lock.json
shared-module/package-lock.json
is excluded by!**/package-lock.json
shared-module/packages/common/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (25)
services/cms/package.json
(1 hunks)services/course-material/package.json
(1 hunks)services/course-material/src/components/ContentRenderer/moocfi/ExerciseBlock/index.tsx
(3 hunks)services/course-material/src/pages/[organizationSlug]/exams/[id].tsx
(5 hunks)services/course-material/src/pages/[organizationSlug]/exams/testexam/[id].tsx
(4 hunks)services/course-material/types/DeprecatedGutenbergBlockAttributes.guard.ts
(61 hunks)services/course-material/types/DeprecatedGutenbergBlockAttributes.ts
(56 hunks)services/course-material/types/GutenbergBlockAttributes.guard.ts
(19 hunks)services/course-material/types/GutenbergBlockAttributes.ts
(10 hunks)services/example-exercise/package.json
(1 hunks)services/main-frontend/package.json
(1 hunks)services/quizzes/package.json
(1 hunks)services/tmc/package.json
(1 hunks)shared-module/package.json
(1 hunks)shared-module/packages/common/package.json
(1 hunks)shared-module/packages/common/src/locales/ar/course-material.json
(2 hunks)shared-module/packages/common/src/locales/ar/main-frontend.json
(1 hunks)shared-module/packages/common/src/locales/ar/quizzes.json
(1 hunks)shared-module/packages/common/src/locales/en/course-material.json
(1 hunks)shared-module/packages/common/src/locales/fi/course-material.json
(2 hunks)shared-module/packages/common/src/locales/fi/quizzes.json
(1 hunks)shared-module/packages/common/src/locales/uk/course-material.json
(1 hunks)shared-module/packages/common/src/locales/uk/main-frontend.json
(1 hunks)shared-module/packages/common/src/locales/uk/quizzes.json
(1 hunks)shared-module/packages/common/src/utils/time.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- shared-module/package.json
- services/tmc/package.json
- services/quizzes/package.json
- services/course-material/package.json
- services/example-exercise/package.json
- services/main-frontend/package.json
🔇 Additional comments (22)
shared-module/packages/common/src/utils/time.ts (1)
40-48
: LGTM! Well-structured date formatting function.The function properly handles null/undefined inputs and uses a localized datetime format. The delegation of timezone conversion to a separate function follows good separation of concerns.
services/course-material/src/pages/[organizationSlug]/exams/testexam/[id].tsx (1)
311-311
: LGTM! Good defensive programming.The additional condition
secondsLeft >= 0
prevents showing the warning message after the exam time is up.services/course-material/src/pages/[organizationSlug]/exams/[id].tsx (1)
Line range hint
714-746
: LGTM! Improved exam state handling.The changes correctly handle the exam ended state and provide appropriate UI feedback to users.
services/course-material/src/components/ContentRenderer/moocfi/ExerciseBlock/index.tsx (1)
372-372
: LGTM! Better text wrapping behavior.Using
overflow-wrap: anywhere
instead ofword-break
provides better control over text wrapping and is the modern approach.shared-module/packages/common/package.json (1)
40-40
: LGTM! Verify timezone handling implementation.The addition of
date-fns-tz
alongsidedate-fns
is appropriate for timezone-aware date handling. Both packages are at compatible versions.Run this script to verify the timezone handling implementation:
✅ Verification successful
Timezone handling properly implemented and widely used
The
date-fns-tz
dependency is well-integrated in the shared module's time utilities and is being used across multiple services for consistent timezone handling. The implementation inshared-module/packages/common/src/utils/time.ts
provides robust timezone support with proper error handling.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for timezone-related function usage # Expected: Find usage of date-fns-tz functions in the codebase rg -l "from 'date-fns-tz'"Length of output: 34
Script:
#!/bin/bash # Try different patterns to find date-fns-tz usage echo "=== Searching for any mention of date-fns-tz ===" rg -i "date-fns-tz" echo -e "\n=== Searching for common timezone functions ===" rg -i "formatInTimeZone|getTimezoneOffset|zonedTimeToUtc|utcToZonedTime" echo -e "\n=== Checking for require statements ===" rg "require.*date-fns-tz"Length of output: 4861
Script:
#!/bin/bash # Check the implementation in time.ts echo "=== Content of time.ts ===" cat "shared-module/packages/common/src/utils/time.ts" echo -e "\n=== Files importing time.ts ===" rg -l "from ['\"].*utils/time['\"]"Length of output: 3057
services/cms/package.json (2)
16-25
: LGTM! Dependencies updated to latest compatible versions.The updates maintain compatibility while bringing in latest improvements:
- UI packages (@emotion, @fontsource) updated to latest minor versions
- WordPress packages updated to latest compatible versions
- Development tools and utilities updated
Also applies to: 28-42, 44-44, 47-47, 49-52, 55-57, 59-62
67-67
: LGTM! Development dependencies updated to latest versions.Development dependencies have been updated to their latest compatible versions, which should bring improvements in the development workflow.
Also applies to: 71-73, 77-77, 80-84, 87-87
shared-module/packages/common/src/locales/ar/quizzes.json (1)
50-50
: LGTM! Arabic translation added for markdown preview feature.The translation "إخفاء معاينة التنسيق" is properly formatted and consistent with the UI terminology used in other translations.
shared-module/packages/common/src/locales/fi/quizzes.json (1)
50-50
: LGTM! Finnish translation added for markdown preview feature.The translation "Piilota markdown-esikatselu" is consistent with existing markdown-related terminology in the Finnish localization.
shared-module/packages/common/src/locales/uk/quizzes.json (1)
50-50
: LGTM! Ukrainian translation added for markdown preview feature.The translation "Приховати попередній перегляд markdown" is consistent with existing markdown-related terminology in the Ukrainian localization.
shared-module/packages/common/src/locales/ar/course-material.json (1)
74-74
: Well-structured Arabic translations!The new translations are accurate, maintain consistent tone, and properly handle right-to-left formatting.
Also applies to: 128-129
shared-module/packages/common/src/locales/en/course-material.json (1)
74-74
: Clear and consistent English message!The new exam completion message follows the established style and is properly formatted.
shared-module/packages/common/src/locales/fi/course-material.json (1)
75-75
: Well-structured Finnish translations!The new translations accurately convey the meaning while maintaining proper Finnish language conventions.
Also applies to: 130-131
shared-module/packages/common/src/locales/uk/course-material.json (1)
75-75
: Well-structured Ukrainian translation!The new exam completion message is accurately translated while maintaining proper Ukrainian language conventions.
shared-module/packages/common/src/locales/ar/main-frontend.json (2)
303-303
: Well-structured Arabic translation!The new marketing consent label is accurately translated while maintaining proper Arabic language conventions.
Line range hint
74-75
: Missing marketing consent translations in English and Ukrainian files.The marketing consent related strings (
marketing-consent-checkbox-text
andmarketing-consent-privacy-policy-checkbox-text
) are present in Arabic and Finnish files but missing from English and Ukrainian files. Please ensure all language files have consistent translations.shared-module/packages/common/src/locales/uk/main-frontend.json (1)
304-304
: LGTM! The new translation entry is well-formatted.The added Ukrainian translation for the marketing consent label is grammatically correct and follows the existing translation patterns in the file.
services/course-material/types/GutenbergBlockAttributes.ts (2)
Line range hint
1-8
: Note: This is an auto-generated file.The file header indicates this is auto-generated. Any manual changes will be overwritten when running
bin/extract-gutenberg-types
.Verify the auto-generation script is up to date:
✅ Verification successful
Auto-generation setup is properly maintained
The auto-generation script
bin/extract-gutenberg-types
exists and was last modified on January 15, 2025, indicating it's actively maintained. The review comment's warning about manual changes being overwritten is accurate.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the auto-generation script exists and when it was last modified # Test: Ensure the script exists and is maintained. Expect: Script exists with recent modifications. ls -l bin/extract-gutenberg-typesLength of output: 112
272-272
:⚠️ Potential issueBreaking change:
linkDestination
is now required inImageAttributes
.The
linkDestination
property has been changed from optional to required, which is a breaking change. Ensure that all image blocks in the codebase provide this property.Run this script to find potentially affected image blocks:
services/course-material/types/DeprecatedGutenbergBlockAttributes.ts (1)
89-102
: New deprecated interface added:ButtonDeprecated12Attributes
.The addition of a new deprecated interface suggests a version migration. Consider documenting the migration path from this version to the current non-deprecated interface.
Check for other deprecated button versions:
services/course-material/types/GutenbergBlockAttributes.guard.ts (1)
Line range hint
1-916
: Type guards implementation looks good!The type guards correctly validate all new properties with consistent patterns. The implementation maintains type safety and follows best practices.
services/course-material/types/DeprecatedGutenbergBlockAttributes.guard.ts (1)
Line range hint
1-2170
: Type guards for deprecated attributes look good!The implementation correctly validates all properties, maintains consistency with non-deprecated guards, and follows the same patterns.
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
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: 0
🧹 Nitpick comments (1)
system-tests/playwright.config.ts (1)
22-24
: Standardize locale format across configurations.There's potential redundancy between the new
env.lang: "en_US"
and the existingcontextOptions.locale: "en-US"
. Consider:
- The formats are inconsistent ("en_US" vs "en-US")
- Having both configurations might lead to unexpected behavior
Standardize the locale format and consider consolidating the language configuration:
launchOptions: { args: [ "--font-render-hinting=none", "--disable-skia-runtime-opts", "--disable-font-subpixel-positioning", "--disable-lcd-text", ], - env: { - lang: "en_US", - }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
system-tests/src/__screenshots__/sidebar-toggle.spec.ts/block-list-desktop-regular.png
is excluded by!**/*.png
system-tests/src/__screenshots__/sidebar-toggle.spec.ts/block-menu-desktop-regular.png
is excluded by!**/*.png
system-tests/src/__screenshots__/sidebar-toggle.spec.ts/block-properties-desktop-regular.png
is excluded by!**/*.png
📒 Files selected for processing (2)
system-tests/playwright.config.ts
(1 hunks)system-tests/src/tests/gutenberg/spacer-not-breaking-editing.spec.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: headless_lms
- GitHub Check: build-and-deploy
- GitHub Check: build
🔇 Additional comments (2)
system-tests/playwright.config.ts (1)
16-21
: Well-documented anti-aliasing configuration!The anti-aliasing configuration is properly documented with a reference to the GitHub issue, and the flags are correctly set to ensure consistent font rendering across different environments, which is crucial for reliable screenshot testing.
system-tests/src/tests/gutenberg/spacer-not-breaking-editing.spec.ts (1)
52-52
: LGTM! Improved selector specificity.The change from
getByRole
togetByLabel
with exact matching improves the selector's precision and reduces potential ambiguity in element selection.Let's verify this selector works consistently with the updated Playwright version:
✅ Verification successful
Selector change is safe and improves element targeting
The change from
getByRole
togetByLabel
with exact matching is an isolated modification that doesn't impact other tests. The new selector provides better specificity and follows Playwright's best practices.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the selector usage pattern across tests # Look for similar selector patterns to ensure consistency # Search for getByLabel usage patterns rg "getByLabel.*Options.*exact:\s*true" --type ts # Search for potential duplicate "Options" labels that could conflict ast-grep --pattern 'aria-label="Options"'Length of output: 224
Script:
#!/bin/bash # Search for any Options-related selectors with various cases rg -i "options.*(?:getByLabel|getByRole|getByText)" --type ts -A 2 # Search for button interactions that might be related rg "\.getByRole\(.*button.*\)" --type ts -A 2 # Look for any aria-label variations of Options rg -i 'aria-label=["'\''].*options.*["'\'']' --type tsLength of output: 65766
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: 0
🧹 Nitpick comments (2)
system-tests/src/tests/exams/teacher-can-test-exam.spec.ts (2)
79-79
: Standardize submission confirmation checks across the test.Good addition of the submission confirmation check. However, there's an inconsistency in how submission confirmations are handled later in the test. Consider standardizing the approach:
- await page.getByText("Your submission has been saved.").waitFor() + await page.getByText("Your submission has been").waitFor()And update the later check to match:
- await page.getByText("Your submission has been").isVisible() + await page.getByText("Your submission has been").waitFor()This standardization will make the test more maintainable and reduce potential points of failure.
Line range hint
1-124
: Consider improving test reliability and maintainability.While reviewing the submission confirmation changes, I noticed some opportunities to improve the overall test:
- Replace explicit timeouts with proper wait conditions:
// Instead of: await page.waitForTimeout(100) // Consider using: await expect(page.locator(...)).toBeVisible({ timeout: 5000 })
- Extract magic numbers into named constants:
const ANIMATION_COMPLETION_TIMEOUT = 100 const DEFAULT_ASSERTION_TIMEOUT = 5000
- Make text assertions more specific by using exact matches where appropriate:
// Instead of: await page.getByText("Your submission has been").waitFor() // Consider: await page.getByText("Your submission has been saved.", { exact: true }).waitFor()These changes would make the test more reliable and easier to maintain. Would you like me to propose a more detailed refactoring?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
system-tests/src/tests/exams/teacher-can-test-exam.spec.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build
- GitHub Check: headless_lms
- GitHub Check: build-and-deploy
- GitHub Check: code-style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.husky/lint-staged.branch-ready.config.js
(1 hunks)system-tests/src/tests/gutenberg/spacer-not-breaking-editing.spec.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- system-tests/src/tests/gutenberg/spacer-not-breaking-editing.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: build-and-deploy
- GitHub Check: headless_lms
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: 0
🧹 Nitpick comments (2)
services/quizzes/src/components/exercise-service-views/Renderer.tsx (1)
45-45
: LGTM! Consider type safety improvement.The addition of data-view-type attributes is well-implemented and consistent across all view types. This change improves testability by providing reliable selectors for the testing framework.
Consider creating a type for the view types to ensure consistency:
type ViewType = "answer-exercise" | "view-submission" | "exercise-editor";This type could be shared between the component and the testing utilities.
Also applies to: 56-56, 68-68
system-tests/src/utils/iframeLocators.ts (1)
79-90
: LGTM! Well-documented testing utility.The new waitForViewType function is well-implemented with proper type safety and documentation. It integrates seamlessly with the existing locator utilities.
Consider adding a timeout parameter with a default value for cases where the view might take longer to load:
export async function waitForViewType( locator: Locator, viewType: "answer-exercise" | "view-submission" | "exercise-editor", timeout?: number, ) { await locator.locator(`[data-view-type="${viewType}"]`).waitFor({ timeout }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
services/example-exercise/src/components/Renderer.tsx
(2 hunks)services/quizzes/src/components/exercise-service-views/Renderer.tsx
(3 hunks)system-tests/src/tests/exams/teacher-can-test-exam.spec.ts
(3 hunks)system-tests/src/utils/iframeLocators.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- system-tests/src/tests/exams/teacher-can-test-exam.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build
- GitHub Check: build-and-deploy
- GitHub Check: headless_lms
🔇 Additional comments (2)
services/example-exercise/src/components/Renderer.tsx (2)
34-34
: LGTM! Consistent implementation across services.The addition of data-view-type attributes mirrors the implementation in the quizzes service, maintaining consistency across the codebase.
Also applies to: 42-42, 54-54
Line range hint
1-1
: Verify PR scope and title.The changes reviewed focus on improving the testing infrastructure by adding data-view-type attributes and corresponding test utilities. However, the PR is titled "Update deps 2025-01". Could you clarify if there are additional dependency updates in other files, or should the PR title be updated to better reflect these testing improvements?
✅ Verification successful
PR title accurately reflects the changes
The PR title "Update deps 2025-01" is appropriate as it contains comprehensive dependency updates across all services, including:
- Next.js upgrades to v15.1.4
- React updates to v18.3.1/v19.0.0
- TypeScript updates to v5.7.3
- Various utility library updates
The changes to the testing infrastructure are part of the necessary adjustments to support these dependency updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for dependency-related changes in package.json files echo "Checking for dependency updates in package.json files..." git diff origin/main | grep -A 5 -B 5 "package.json" | grep "[-+].*version" # Check for changes in other dependency files echo "Checking for changes in other dependency files..." git diff origin/main | grep -A 5 -B 5 "yarn.lock\|package-lock.json\|pnpm-lock.yaml"Length of output: 778
Script:
#!/bin/bash # First, let's check git information echo "=== Git Information ===" git branch git remote -v # Find all package.json files echo -e "\n=== Package Files ===" fd package.json # Check content of package.json files echo -e "\n=== Package Content ===" fd package.json --exec cat {}Length of output: 24730
Summary by CodeRabbit
Release Notes
Dependencies
date-fns-tz
package for improved time zone handling.Localization
Exam Experience
Block Attributes
Styling
Kubernetes
Testing
These updates focus on improving user experience, maintaining dependencies, and enhancing type safety across the project.