Merged
Conversation
johan-bell
reviewed
Apr 8, 2026
Comment on lines
+20
to
+28
| const s3Endpoint = process.env.S3_ENDPOINT || "127.0.0.1"; | ||
| const s3Port = process.env.S3_PORT || "9000"; | ||
| const s3AccessKey = process.env.S3_ACCESS_KEY || "minio"; | ||
| const s3SecretKey = process.env.S3_SECRET_KEY || "minio123"; | ||
| const s3UseSsl = process.env.S3_USE_SSL === "true"; | ||
| const s3Protocol = s3UseSsl ? "https" : "http"; | ||
| const s3BaseUrl = `${s3Protocol}://${s3Endpoint}:${s3Port}`; | ||
| const s3PublicUrl = process.env.S3_PUBLIC_URL || "http://localhost:9000"; | ||
|
|
Collaborator
There was a problem hiding this comment.
We have duplicate declaration here. check ligne 181 - 188
Comment on lines
+181
to
+189
| const s3Endpoint = process.env.S3_ENDPOINT || "127.0.0.1"; | ||
| const s3Port = process.env.S3_PORT || "9000"; | ||
| const s3AccessKey = process.env.S3_ACCESS_KEY || "minio"; | ||
| const s3SecretKey = process.env.S3_SECRET_KEY || "minio123"; | ||
| const s3UseSsl = process.env.S3_USE_SSL === "true"; | ||
| const s3Protocol = s3UseSsl ? "https" : "http"; | ||
| const s3BaseUrl = `${s3Protocol}://${s3Endpoint}:${s3Port}`; | ||
| const s3PublicUrl = process.env.S3_PUBLIC_URL || "http://localhost:9000"; | ||
|
|
Comment on lines
+539
to
+547
| const s3Endpoint = process.env.S3_ENDPOINT || "127.0.0.1"; | ||
| const s3Port = process.env.S3_PORT || "9000"; | ||
| const s3AccessKey = process.env.S3_ACCESS_KEY || "minio"; | ||
| const s3SecretKey = process.env.S3_SECRET_KEY || "minio123"; | ||
| const s3UseSsl = process.env.S3_USE_SSL === "true"; | ||
| const s3Protocol = s3UseSsl ? "https" : "http"; | ||
| const s3BaseUrl = `${s3Protocol}://${s3Endpoint}:${s3Port}`; | ||
| const s3PublicUrl = process.env.S3_PUBLIC_URL || "http://localhost:9000"; | ||
|
|
Comment on lines
+19
to
+27
| const s3Endpoint = process.env.S3_ENDPOINT || "127.0.0.1"; | ||
| const s3Port = process.env.S3_PORT || "9000"; | ||
| const s3AccessKey = process.env.S3_ACCESS_KEY || "minio"; | ||
| const s3SecretKey = process.env.S3_SECRET_KEY || "minio123"; | ||
| const s3UseSsl = process.env.S3_USE_SSL === "true"; | ||
| const s3Protocol = s3UseSsl ? "https" : "http"; | ||
| const s3BaseUrl = `${s3Protocol}://${s3Endpoint}:${s3Port}`; | ||
| const s3PublicUrl = process.env.S3_PUBLIC_URL || "http://localhost:9000"; | ||
|
|
Collaborator
There was a problem hiding this comment.
May be make a reusable function or file that will be used in the tests ?
Comment on lines
+25
to
+34
| it("should include hlsUrl when both mediaType and hlsUrl are provided", () => { | ||
| const result = createUploadData(mockFile, "default", { | ||
| mediaType: MediaType.Video, | ||
| hlsUrl: "https://example.com/stream.m3u8", | ||
| languageId: "lang-2", | ||
| }); | ||
| expect(result.hlsUrl).toBe("https://example.com/stream.m3u8"); | ||
| expect(result.mediaType).toBe(MediaType.Video); | ||
| expect(result.languageId).toBe("lang-2"); | ||
| }); |
Collaborator
There was a problem hiding this comment.
What are we really testing here ?
Comment on lines
+1
to
+20
| import { ChangeReqAckDto } from "./ChangeReqAckDto"; | ||
| import { AckStatus } from "../enums"; | ||
|
|
||
| describe("ChangeReqAckDto", () => { | ||
| it("should create an instance with ack status", () => { | ||
| const dto = new ChangeReqAckDto(); | ||
| dto.ack = AckStatus.Accepted; | ||
| expect(dto.ack).toBe(AckStatus.Accepted); | ||
| }); | ||
|
|
||
| it("should support optional message and warnings", () => { | ||
| const dto = new ChangeReqAckDto(); | ||
| dto.ack = AckStatus.Rejected; | ||
| dto.message = "Insufficient permissions"; | ||
| dto.warnings = ["Image upload failed"]; | ||
| dto.docs = []; | ||
| expect(dto.message).toBe("Insufficient permissions"); | ||
| expect(dto.warnings).toHaveLength(1); | ||
| }); | ||
| }); |
Collaborator
There was a problem hiding this comment.
Not sure if this test is needed
Comment on lines
+35
to
+43
| it("should create instance with all properties", () => { | ||
| const dto = createValid(); | ||
| dto.memberOf = ["group-1"]; | ||
| dto.newMemberOf = ["group-2"]; | ||
| dto.language = "lang-1"; | ||
| dto.slug = "my-slug"; | ||
| expect(dto.memberOf).toEqual(["group-1"]); | ||
| expect(dto.newMemberOf).toEqual(["group-2"]); | ||
| }); |
|
|
||
| service = (await createTestingModule("search-service")).dbService; | ||
| searchService = new SearchService(undefined, service); | ||
| searchService = new SearchService({ error: jest.fn(), warn: jest.fn() } as any, service); |
Collaborator
There was a problem hiding this comment.
is the test failng here before the change ?
Comment on lines
+7
to
+15
| const s3Endpoint = process.env.S3_ENDPOINT || "127.0.0.1"; | ||
| const s3Port = process.env.S3_PORT || "9000"; | ||
| const s3AccessKey = process.env.S3_ACCESS_KEY || "minio"; | ||
| const s3SecretKey = process.env.S3_SECRET_KEY || "minio123"; | ||
| const s3UseSsl = process.env.S3_USE_SSL === "true"; | ||
| const s3Protocol = s3UseSsl ? "https" : "http"; | ||
| const s3BaseUrl = `${s3Protocol}://${s3Endpoint}:${s3Port}`; | ||
| const s3PublicUrl = process.env.S3_PUBLIC_URL || "http://localhost:9000"; | ||
|
|
ivanslabbert
approved these changes
Apr 9, 2026
Contributor
ivanslabbert
left a comment
ivanslabbert
left a comment
There was a problem hiding this comment.
Approved, but check comments from Johan
MrDirkelz
force-pushed
the
1464-api-implement-full-test-coverage
branch
from
2986cad to
2b4ecd8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.