Release 8.1.0 - #300
Conversation
….1.1 Issue #ED-619 feat: Update dependencies and use dayjs instead of momentjs
Issue #0000 fix: Updated the cs-telemetry services to log the search events
…-6.0.0 Issue #ED-1950 fix: Updated typescript version and fix on typescript …
…-6.0.0-2131 Issue #ED-2131 fix: fix on ts-loader on publish npm
…ase-7.0.0 Issue #SB-ED-3073 feat: Added framework config implementation in framework service.
…ase-7.0.0 Issue #SB-ED-3073 chore: Updated package version.
…ase-7.0.0 Issue #SB-ED-3073 fix: Updated getCategoryMap issue.
…ase-7.0.0 Issue #SB-ED-3073 chore: Updated index in getFrameworkConfig.
…ase-7.0.0 Issue #SB-ED-3073 chore: Updated requiredCategories attribute.
SBCOSS-473: Added GitHub Actions for pull request yml file
SBCOSS-474: Added GitHib actions for publishing to NPM
#SBCOSS-557 - removed bmgs hardcoding
#SBCOSS-557: Fix jsonld.js global object dependency issue
SB-I367: cryptold version change to support RSAKeyPair constructor.
SB-I367: generate fresh lock file on installation
SB-I367: version of jsonld signatures reverted to v7
SB-I367: jsold versiosn changes for 8.1.0 version
WalkthroughRemoves CircleCI configuration and introduces GitHub Actions workflows for PR and release builds. Updates dependencies significantly, refactors content and course service interfaces, expands framework service with configuration management, adds telemetry search methods, updates test fixtures with new identifiers, and extends webpack configuration with Babel support and runtime polyfills. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/utilities/certificate/certificate-verifier.ts (1)
94-105: Remove debug console.log statements before release.Lines 94 and 105 contain
console.logstatements with TODO comments. These debug artifacts should be removed for production code to avoid exposing internal details and cluttering logs.Proposed fix
- console.log('====> host ', this.host); // TODO: log! const apiRequest: CsRequest = new CsRequest.Builder() .withHost('https://') .withType(CsHttpRequestType.GET) .withPath(url.split('//')[1]) .withBearerToken(false) .withHeaders({ 'Origin': this.host }) .withUserToken(false) .build(); - console.log('====> apiRequest ', apiRequest); // TODO: log! const jsonResp = await this.httpService.fetch(apiRequest)src/cs-module.ts (1)
79-88: Remove duplicate CsCertificateServiceConfig interface declaration.The
CsCertificateServiceConfiginterface is declared twice (lines 79-83 and 84-88) with identical fields. This is redundant and may cause confusion or compilation issues.🔎 Proposed fix
export interface CsCertificateServiceConfig { apiPath: string; apiPathLegacy?: string; rcApiPath: string; } -export interface CsCertificateServiceConfig { - apiPath: string; - apiPathLegacy?: string; - rcApiPath: string; -} export interface CsFrameworkConfig { apiPath: string; framework?: Framework; }
🧹 Nitpick comments (12)
src/telemetry/implementation/telemetry-service-Impl.ts (1)
72-74: Consider throwing an error for unimplemented stub.The empty implementation is consistent with most other
...Withtelemetry methods in this file. However,raiseEndTelemetryWith(lines 15-17) throws an explicit "Method not implemented" error, which provides clearer feedback to callers.🔎 Suggested implementation for clarity
public raiseSearchTelemetryWith(cdata: Array<ICDataEntry>, env: string, edata: any, telemetryObject?: ITelemetryObject) { - + throw new Error('Method not implemented.'); }src/utilities/certificate/certificate-verifier.ts (1)
4-4: Remove unusedEd25519KeyPairimport.
Ed25519KeyPairis destructured fromcrypto-ldbut never used in this file. The switch to CommonJSrequirealigns with the webpack external configuration.Proposed fix
-const {RSAKeyPair, Ed25519KeyPair} = require('crypto-ld'); +const {RSAKeyPair} = require('crypto-ld');README.md (1)
27-40: Add language specifiers to fenced code blocks.Per markdown best practices (MD040), fenced code blocks should specify a language for proper syntax highlighting. Based on static analysis hints.
Proposed fix
-``` +```bash git clone https://github.com/your-username/sunbird-client-service.git cd sunbird-client-service
- Install dependencies:
-
+bash
npm i4. To run the project -``` +```bash npm run build</details> </blockquote></details> <details> <summary>.github/workflows/pull_request.yml (1)</summary><blockquote> `43-44`: **Remove redundant webpack installation.** Webpack and webpack-cli are already listed as devDependencies in package.json (lines 56-57) and will be installed by `npm i`. This separate installation step is unnecessary. <details> <summary>🔎 Proposed fix</summary> ```diff - - name: Install webpack - run: npm install --save-dev webpack webpack-cli - - name: Install SonarQube scanner run: npm install -g sonarqube-scanner.github/workflows/on_push.yml (1)
29-30: Remove redundant webpack installation.Webpack and webpack-cli are already devDependencies and will be installed by
npm install. This step is unnecessary.🔎 Proposed fix
- name: Install dependencies run: | npm install - - - name: Install webpack - run: npm install --save-dev webpack webpack-clisrc/services/framework/implementation/cs-framework-config-bloc.ts (2)
18-20: Public getter exposes internal state for mutation.The
configgetter returns a direct reference to the internalcategoryConfigobject, allowing external code to mutate it without going throughupdateState. Consider returning a defensive copy or making it readonly.🔎 Proposed fix
public get config() { - return this.categoryConfig; + return { ...this.categoryConfig }; }Alternatively, if you want to allow read-only access without copying:
public get config(): Readonly<{ [frameworkId: string]: Array<FrameworkConfig> }> { return this.categoryConfig; }
22-28: Consider adding method documentation.The
updateStateanddisposemethods lack documentation explaining their purpose and usage. Adding JSDoc comments would improve maintainability.🔎 Suggested documentation
+ /** + * Updates the framework configuration for a given framework ID. + * @param frameWorkId The framework identifier + * @param config Array of framework configuration objects + */ updateState(frameWorkId: string, config: Array<FrameworkConfig>) { this.categoryConfig[frameWorkId] = config; } + /** + * Clears all cached framework configurations. + */ dispose() { this.categoryConfig = {} }src/services/framework/implementation/framework-service-impl.spec.ts (1)
62-131: LGTM with minor suggestion.The new test suite for
getFrameworkConfigprovides good coverage of different scenarios. However, lines 71 and 96 have duplicate test descriptions which could cause confusion.🔎 Suggested improvement for test clarity
- it('should be able fetch the framework config if form config is available', (done) => { + it('should be able fetch the framework config if form config is available and form service returns error', (done) => { const sampleFrameWork: Framework = { name: "", identifier: "", categories: [{code: "SAMPLE_CODE", name: "SAMPLE_NAME", identifier:"SAMPLE_ID", description:"", index:1,status:""}]} mockFormService.getForm = jest.fn(() => {This distinguishes the error-handling test case from the success case on line 71.
src/services/framework/interface/cs-framework-service.ts (1)
21-24: Simplify return type to improve type safety.The return type
Observable<FrameworkConfig[] | Array<any>>is redundant sinceFrameworkConfig[]is already an array. TheArray<any>fallback defeats the purpose of type safety. Consider using onlyObservable<FrameworkConfig[]>or documenting why the any type is needed.🔎 Proposed fix
If you always return FrameworkConfig arrays:
- getFrameworkConfig(frameworkId: string, frameworkConfig?: CsFrameworkConfig, formConfig?: CsFormConfig): Observable<FrameworkConfig[] | Array<any>>; + getFrameworkConfig(frameworkId: string, frameworkConfig?: CsFrameworkConfig, formConfig?: CsFormConfig): Observable<FrameworkConfig[]>;If you need flexibility for different return types, consider a generic or union of specific types:
getFrameworkConfig(frameworkId: string, frameworkConfig?: CsFrameworkConfig, formConfig?: CsFormConfig): Observable<FrameworkConfig[]>;src/services/content/utilities/content-group-generator/cs-contents-group-generator.ts (1)
190-190: Type cast may be unsafe for symbol keys.Casting
groupBy as stringassumes it's always a string key. IfgroupBycould be a symbol (which is a validkeyof Content), this cast would be incorrect. Consider adding a runtime check or constraining the generic type.🔎 Suggested improvement
return { - name: groupBy as string, + name: String(groupBy), sections, combination: resultingCombination };Or constrain the type parameter:
static generate(config: { // ... other config - groupBy: keyof Content, + groupBy: string & keyof Content, // ... }): CsContentSection {src/services/framework/implementation/framework-service-impl.ts (2)
50-53: Error silently swallowed without logging.The caught error
eis discarded without any logging or telemetry. This could make debugging production issues difficult when the form service fails.🔎 Proposed fix
catchError((e) => { + console.warn('Failed to fetch form config, falling back to framework transform:', e); return this.transformFramework(frameworkId, frameworkConfig?.apiPath ?? ""); - }))
79-83: LGTM!The transformation logic correctly maps framework categories to the config format. Minor formatting: there's an extra blank line at line 81.
🔎 Optional: Remove extra blank line
/** @internal */ transformCategoriesToConfig(categories: FrameworkCategory[]): FrameworkConfig[] { - return categories.map(({ index, code, name }) => ({ code: code, label: name, identifier: `fwCategory${index}`, index: index, placeHolder: `Select ${code}`, name })) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/services/content/utilities/content-group-generator/__snapshots__/cs-contents-group-generator.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (21)
.circleci/config.yml.github/workflows/on_push.yml.github/workflows/pull_request.ymlREADME.mdpackage.jsonsrc/cs-module.tssrc/models/channel/index.tssrc/models/content/index.tssrc/services/content/utilities/content-group-generator/cs-contents-group-generator.spec.data.tssrc/services/content/utilities/content-group-generator/cs-contents-group-generator.tssrc/services/course/interface/cs-course-service.tssrc/services/framework/implementation/cs-framework-config-bloc.tssrc/services/framework/implementation/framework-service-impl.spec.tssrc/services/framework/implementation/framework-service-impl.tssrc/services/framework/interface/cs-framework-service.tssrc/services/group/activity/implementation/group-activity-service-impl.spec.tssrc/services/group/implementation/group-service-impl.tssrc/telemetry/implementation/telemetry-service-Impl.tssrc/telemetry/interface/cs-telemetry-service.tssrc/utilities/certificate/certificate-verifier.tswebpack.config.js
💤 Files with no reviewable changes (1)
- .circleci/config.yml
🧰 Additional context used
🧬 Code graph analysis (7)
src/telemetry/interface/cs-telemetry-service.ts (1)
src/telemetry/interface/cs-telemetry-request.ts (2)
ICDataEntry(31-34)ITelemetryObject(14-19)
src/services/framework/implementation/cs-framework-config-bloc.ts (2)
src/services/framework/interface/cs-framework-service.ts (1)
FrameworkConfig(9-16)src/cs-module.ts (1)
config(157-159)
src/cs-module.ts (2)
src/models/channel/index.ts (1)
Framework(1-11)src/services/form/interface/cs-form-service.ts (1)
FormParams(5-12)
src/telemetry/implementation/telemetry-service-Impl.ts (1)
src/telemetry/interface/cs-telemetry-request.ts (2)
ICDataEntry(31-34)ITelemetryObject(14-19)
src/services/framework/interface/cs-framework-service.ts (2)
src/cs-module.ts (3)
CsFrameworkServiceConfig(58-60)CsFrameworkConfig(90-93)CsFormConfig(95-98)src/models/channel/index.ts (1)
Framework(1-11)
src/services/content/utilities/content-group-generator/cs-contents-group-generator.ts (1)
src/models/content/index.ts (1)
Content(67-69)
src/services/framework/implementation/framework-service-impl.spec.ts (6)
src/cs-module.ts (1)
frameworkService(173-175)src/services/framework/interface/cs-framework-service.ts (1)
CsFrameworkService(18-26)src/core/http-service/interface/cs-http-service.ts (1)
CsHttpService(7-12)src/services/form/interface/cs-form-service.ts (1)
CsFormService(14-16)src/models/channel/index.ts (1)
Framework(1-11)src/core/http-service/interface/cs-response.ts (1)
CsResponse(10-58)
🪛 actionlint (1.7.9)
.github/workflows/pull_request.yml
25-25: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
38-38: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 Gitleaks (8.30.0)
src/services/group/activity/implementation/group-activity-service-impl.spec.ts
[high] 1339-1339: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.18.1)
README.md
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
34-34: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
38-38: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (22)
src/models/channel/index.ts (1)
1-62: File structure is sound; trailing newline follows best practices.The addition of a trailing newline at EOF aligns with the standard convention endorsed by linting tools. The interfaces are well-typed with appropriate optional properties and consistent naming conventions. No functional or API surface changes detected.
src/telemetry/interface/cs-telemetry-service.ts (1)
26-27: LGTM! Consistent API extension.The new search telemetry methods follow the established naming and signature conventions of existing telemetry methods in this interface.
src/telemetry/implementation/telemetry-service-Impl.ts (1)
66-70: Implementation is correct and properly implements the interface contract.The Search telemetry method is used to capture the search state when search is triggered for content, item, assets etc. The implementation correctly follows the established pattern with proper initialization guards. The
raiseSearchTelemetrymethod is properly declared in the abstract interface (lines 26-27) and the implementation at lines 66-70 matches the interface contract, consistent with other telemetry methods like interact and impression.src/services/course/interface/cs-course-service.ts (2)
5-10: API change:filtersnow accepts any key-value pairs.The explicit filter fields have been replaced with an index signature
[key: string]: any. This increases flexibility but removes compile-time type checking for filter properties. Consumers using previously defined filter fields will still work, but typos in filter keys won't be caught at compile time.Ensure downstream consumers are aware of this interface change if they relied on specific filter field autocompletion or type checking.
18-24: API change:contentIdsis now optional.Changing
contentIds: string[]tocontentIds?: string[]means the API will now accept requests without content IDs. Verify this aligns with the backend API behavior—if the server still requirescontentIds, runtime errors could occur.Confirm the backend API supports omitting
contentIdsin the request payload.webpack.config.js (2)
39-41: LGTM! External modules configuration.Adding
crypto-ldandjsonldas externals aligns with the CommonJS require usage incertificate-verifier.ts, ensuring these modules are not bundled but loaded at runtime.
55-64: Babel loader for jsonld-signatures compatibility.The rule specifically includes
jsonld-signaturesfromnode_modulesfor Babel processing, which addresses ES module compatibility. This is a targeted fix for the library's module format.src/services/group/activity/implementation/group-activity-service-impl.spec.ts (2)
875-875: LGTM! Test fixture identifier updates.The batch count key rename from
c_diksha_preprod_private_batch_counttoc_sunbird_preprod_private_batch_countaligns with the broader rebranding in test fixtures.
1337-1345: Test fixture updates for branding consistency.The
appIdchanges fromdikshatosunbirdvariants andaudienceupdates to generic values are consistent with the test data normalization across the PR.Note: The static analysis hint about line 1339 (
lockKey) being a "Generic API Key" is a false positive—this is a UUID used as a test fixture identifier, not an actual API key.src/models/content/index.ts (2)
1-69: API surface change:Contentnow extendsContentBasewith index signature.The refactoring separates typed fields into
ContentBaseand extends it with[key: string]: anyinContent. This pattern enables:
- Type safety for known fields via
ContentBase- Flexibility for dynamic/additional properties via the index signature
Note that fields like
subject,board,medium, andse_*attributes mentioned in the summary as removed will no longer have type definitions, though they can still be accessed via the index signature.Verify that consumers depending on the removed typed fields (e.g.,
subject,board,medium) are updated or can handle the dynamic access pattern.
71-75: LGTM! Transcript interface updated with language field.The addition of
language: stringto theTranscriptinterface provides necessary metadata for transcript content.src/services/content/utilities/content-group-generator/cs-contents-group-generator.spec.data.ts (1)
48-48: LGTM! Test fixture branding updates.The systematic
appIdchanges fromstaging.diksha.portaltostaging.sunbird.portaland keyword substitutions (teacher→user1,student→user2) are consistent test data normalizations across all fixture variants.package.json (2)
3-3: Verify the major version jump follows semantic versioning.The version jumps from 5.0.1 to 8.1.4, skipping major versions 6 and 7. Ensure this aligns with your versioning strategy and that all breaking changes are documented.
18-32: Dependency versions are secure and free of known direct vulnerabilities.Verification confirms that
node-fetch(2.7.0),jsonld(5.2.0), andjszip(3.10.1) have no recorded direct security vulnerabilities. However, continue monitoring dependencies as new vulnerabilities may be discovered, particularly for those handling external data likenode-fetchandjszip.src/cs-module.ts (1)
90-98: LGTM! New configuration interfaces align well with the framework service enhancements.The new
CsFrameworkConfigandCsFormConfiginterfaces provide clear structure for the enhanced framework configuration functionality.src/services/framework/interface/cs-framework-service.ts (1)
9-16: LGTM! Well-defined FrameworkConfig interface.The new
FrameworkConfiginterface is clear and properly typed with all necessary fields for framework configuration.src/services/content/utilities/content-group-generator/cs-contents-group-generator.ts (3)
73-73: LGTM! Good defensive programming.The fallback to an empty array handles cases where
combination[attribute]might be undefined, preventing runtime errors.
98-107: LGTM! Improved null safety in preference extraction.The added guards and intermediate
valuevariable improve null safety and code clarity when extracting user preference values.
136-152: LGTM! Enhanced type safety for grouping logic.The refactored grouping logic with
keyof Contentand explicit searchable key handling improves type safety. The string coercion on line 148 with the 'Other' fallback appropriately handles edge cases..github/workflows/on_push.yml (1)
42-42: The hardcoded package name pattern is correct. For a scoped package like@project-sunbird/client-services,npm pack ./distgenerates a tarball namedproject-sunbird-client-services-8.1.4.tgz(the scope separator is converted to a dash), which matches the glob patternproject-sunbird-client-services-*. No changes needed.Likely an incorrect or invalid review comment.
src/services/framework/implementation/framework-service-impl.ts (2)
15-20: LGTM!The constructor properly injects all required dependencies using the inversify decorators.
60-68: LGTM!The method correctly transforms the config array into a dictionary keyed by identifier. The reduce pattern is appropriate for the expected small size of framework categories.
| if (frameworkConfig?.framework) { | ||
| return of(this.transformCategoriesToConfig(frameworkConfig?.framework.categories!)) |
There was a problem hiding this comment.
Risky non-null assertion on categories.
The non-null assertion categories! assumes categories will always exist when frameworkConfig?.framework is defined. If categories is undefined, transformCategoriesToConfig will fail when iterating.
🔎 Proposed fix
- if (frameworkConfig?.framework) {
- return of(this.transformCategoriesToConfig(frameworkConfig?.framework.categories!))
+ if (frameworkConfig?.framework?.categories) {
+ return of(this.transformCategoriesToConfig(frameworkConfig.framework.categories))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (frameworkConfig?.framework) { | |
| return of(this.transformCategoriesToConfig(frameworkConfig?.framework.categories!)) | |
| if (frameworkConfig?.framework?.categories) { | |
| return of(this.transformCategoriesToConfig(frameworkConfig.framework.categories)) |
🤖 Prompt for AI Agents
In src/services/framework/implementation/framework-service-impl.ts around lines
43-44, the code uses a risky non-null assertion on categories (categories!)
which can throw if categories is undefined; update the branch to guard against
undefined by passing a safe default (e.g., frameworkConfig.framework.categories
?? []) or check for categories presence and return an empty observable/config
when absent, and remove the non-null assertion so transformCategoriesToConfig
always receives a defined array.
| /** @internal */ | ||
| transformFramework(frameworkId: string, apiPath: string): Observable<FrameworkConfig[]> { | ||
| return this.getFramework(frameworkId, {requiredCategories: []}, {apiPath: apiPath}).pipe( | ||
| map((framework) => { | ||
| return this.transformCategoriesToConfig(framework.categories!); | ||
| }) | ||
| ) | ||
| } |
There was a problem hiding this comment.
Add defensive check for framework.categories.
The non-null assertion framework.categories! assumes the API always returns categories. If the framework response lacks categories, this will throw when mapping.
🔎 Proposed fix
/** @internal */
transformFramework(frameworkId: string, apiPath: string): Observable<FrameworkConfig[]> {
return this.getFramework(frameworkId, {requiredCategories: []}, {apiPath: apiPath}).pipe(
map((framework) => {
- return this.transformCategoriesToConfig(framework.categories!);
+ return this.transformCategoriesToConfig(framework.categories ?? []);
})
)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** @internal */ | |
| transformFramework(frameworkId: string, apiPath: string): Observable<FrameworkConfig[]> { | |
| return this.getFramework(frameworkId, {requiredCategories: []}, {apiPath: apiPath}).pipe( | |
| map((framework) => { | |
| return this.transformCategoriesToConfig(framework.categories!); | |
| }) | |
| ) | |
| } | |
| /** @internal */ | |
| transformFramework(frameworkId: string, apiPath: string): Observable<FrameworkConfig[]> { | |
| return this.getFramework(frameworkId, {requiredCategories: []}, {apiPath: apiPath}).pipe( | |
| map((framework) => { | |
| return this.transformCategoriesToConfig(framework.categories ?? []); | |
| }) | |
| ) | |
| } |
🤖 Prompt for AI Agents
In src/services/framework/implementation/framework-service-impl.ts around lines
70 to 77, replace the unsafe non-null assertion on framework.categories with a
defensive check: when the framework returned from getFramework has no categories
(null/undefined), pass an empty array into transformCategoriesToConfig (or
return an empty FrameworkConfig[]), e.g. compute const categories =
framework.categories ?? [] and then call
this.transformCategoriesToConfig(categories) so the method never throws if
categories are missing.
| return dayjs(bLastActivity.slice(0, bLastActivity.lastIndexOf(':'))).valueOf() - | ||
| dayjs(aLastActivity.slice(0, aLastActivity.lastIndexOf(':'))).valueOf(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "group-service-impl.ts" -type fRepository: Sunbird-Ed/sunbird-client-services
Length of output: 135
🏁 Script executed:
wc -l ./src/services/group/implementation/group-service-impl.tsRepository: Sunbird-Ed/sunbird-client-services
Length of output: 139
🏁 Script executed:
head -40 ./src/services/group/implementation/group-service-impl.ts && echo "..." && sed -n '320,340p' ./src/services/group/implementation/group-service-impl.tsRepository: Sunbird-Ed/sunbird-client-services
Length of output: 2764
🏁 Script executed:
cat ./package.json | grep -A 5 -B 5 dayjsRepository: Sunbird-Ed/sunbird-client-services
Length of output: 364
🏁 Script executed:
find . -path ./node_modules -prune -o -name "*.ts" -type f -exec grep -l "updatedOn\|createdOn" {} \; | head -10Repository: Sunbird-Ed/sunbird-client-services
Length of output: 527
🏁 Script executed:
sed -n '1,100p' ./src/models/group/index.ts | head -80Repository: Sunbird-Ed/sunbird-client-services
Length of output: 2006
🏁 Script executed:
grep -A 10 -B 2 "updatedOn\|createdOn" ./src/services/group/implementation/group-service-impl.spec.ts | head -50Repository: Sunbird-Ed/sunbird-client-services
Length of output: 3086
🏁 Script executed:
sed -n '1,200p' ./src/services/group/implementation/group-service-impl.spec.ts | grep -A 30 "sort\|describe"Repository: Sunbird-Ed/sunbird-client-services
Length of output: 7330
🏁 Script executed:
grep -B 5 -A 15 "search(" ./src/services/group/implementation/group-service-impl.spec.ts | grep -A 15 "expect\|should"Repository: Sunbird-Ed/sunbird-client-services
Length of output: 2718
🏁 Script executed:
git log --oneline -n 20 -- ./src/services/group/implementation/group-service-impl.ts 2>/dev/null | head -10Repository: Sunbird-Ed/sunbird-client-services
Length of output: 140
Add a comment explaining the timestamp truncation logic, or remove the truncation if not intentional.
dayjs parses ISO 8601 format timestamps correctly including timezone offsets. The current slicing logic at lastIndexOf(':') truncates the timestamp to minute precision (removing milliseconds and timezone info). For timestamps like 2020-07-16 12:22:45:719+0000, this produces 2020-07-16 12:22:45.
If this precision level is intentional to ignore millisecond differences during sorting, add a clarifying comment. If not intentional, the truncation can be removed—dayjs will parse and compare the full timestamps correctly for accurate sorting.
🤖 Prompt for AI Agents
In src/services/group/implementation/group-service-impl.ts around lines 330-331,
the code slices timestamps at lastIndexOf(':') which strips timezone and
millisecond data before parsing with dayjs; either add a short comment above
this line explaining that truncation to minute precision is intentional to
ignore millisecond/timezone differences when sorting, or remove the slice calls
entirely so dayjs parses full ISO timestamps (including milliseconds/timezone)
for accurate comparison—update tests if any rely on the current behavior.
15 participants
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.