BC-3961-move-s3-client-to-infra #4369
Conversation
| imports: [LoggerModule], | ||
| providers: [S3ClientAdapter], | ||
| }) | ||
| export class S3FileStorageModule { |
There was a problem hiding this comment.
Ist vom Namen schwer von dem FileStorageModule zu unterscheiden oder? Ist es das FileStorageClientModule?
There was a problem hiding this comment.
Ich finde S3 sollte schon im Namen auftauchen, weil es ja dafür spezifisch ist. Vielleicht "S3ClientModule"?
There was a problem hiding this comment.
- FileStorageClientModule ist schon vergeben.
- S3ClientModule habe ich übernommen
| const expectedResponse = FileResponseBuilder.build(fileResponse, fileRecord.getName()); | ||
|
|
||
| spy = jest.spyOn(service, 'downloadFile').mockResolvedValueOnce(fileResponse); | ||
| spy = jest.spyOn(service, 'downloadFile').mockResolvedValueOnce(expectedResponse); |
There was a problem hiding this comment.
Ist das hier richtig so? fileResponse wird nicht mehr benutzt?
There was a problem hiding this comment.
Mapping passiert jetzt dort
| public async downloadFile(schoolId: EntityId, fileRecordId: EntityId, bytesRange?: string): Promise<IGetFile> { | ||
| const pathToFile = createPath(schoolId, fileRecordId); | ||
| const response = await this.storageClient.get(pathToFile, bytesRange); | ||
| public async downloadFile(fileRecord: FileRecord, bytesRange?: string): Promise<GetFileResponse> { |
There was a problem hiding this comment.
Die beiden Methoden downloadFile und download machen doch jetzt das gleiche. Du könntest download einfach rausschmeißen und die bisherigen Aufrufe auf downloadFile ändern.
There was a problem hiding this comment.
Nein, werden für verschiedene use cases benutzt
There was a problem hiding this comment.
Und ich mache kein Refactoring in dem PR! Wobei mir schon paar stellen aufgefallen die ich gerne refactorn würde
There was a problem hiding this comment.
Naja, die Signatur zu ändern, ist eigentlich auch ein Refactoring, halt nur ein kleineres... aber okay...
|
Kudos, SonarCloud Quality Gate passed!
|
Description
Links to Tickets or other pull requests
Changes
Datasecurity
Deployment
New Repos, NPM pakages or vendor scripts
Approval for review
generate-client:serverwas executed in vue frontend and changes were tested and put in a PR with the same branch name.