-
Notifications
You must be signed in to change notification settings - Fork 17
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
BC-5119-Prevent-creating-new-neXboards (Server) #4522
Conversation
apps/server/src/modules/learnroom/controller/rooms.controller.ts
Outdated
Show resolved
Hide resolved
… to ignore LESSON_CONTENT_NEXBOARD.
0049531
to
99428b9
Compare
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.
I think you've missed very important place, which also has some flag to turn on/off copying of the Nexboard, but it's using the general flag for enabling/disabling the Nexboard (which has to stay enabled as we still want to keep the existing Nexboards usable). Please take a look here:
if (element.component === ComponentType.NEXBOARD && nexboardEnabled) { |
apps/server/src/modules/learnroom/service/course-copy.service.ts
Outdated
Show resolved
Hide resolved
53c0136
to
23027d4
Compare
.vscode/launch.default.json
Outdated
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.
Please revert these changes.
.vscode/settings.default.json
Outdated
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.
Please revert these changes.
apps/server/src/modules/learnroom/controller/rooms.controller.ts
Outdated
Show resolved
Hide resolved
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.
Please remember to use prettier
.
fe8e6cb
to
8a4c7ed
Compare
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.
- pls cleanup (see also the linter test)
I am not sure if this is the most elegant way... Rather than delete code and such, I would introduce a feature flag just for the nexboard copy. Perhaps we will need nexboard back in the near future, or some instances will change their mind and will want to keep it including the copying...
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.
revert formating changes on this file
b4f7300
to
8c51e64
Compare
Kudos, SonarCloud Quality Gate passed! |
e65731b
to
1faba72
Compare
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.
plus some cleanup still needs to happen
the .vscode folder should not be chnged
the .DS_Store files - you don't need to touch those
195c5bc
to
e99e428
Compare
.vscode/launch.default.json
Outdated
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.
restore the file
.vscode/settings.default.json
Outdated
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.
restore the file
@@ -46,13 +46,23 @@ export class CourseCopyService { | |||
// copy course and board | |||
const courseCopy = await this.copyCourseEntity({ user, originalCourse, copyName }); | |||
const boardStatus = await this.boardCopyService.copyBoard({ originalBoard, destinationCourse: courseCopy, user }); | |||
const filteredBoardStatus = this.filterOutNeXboardFromCopyStatus(boardStatus); |
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.
here too should use the new feature flag condition...
src/services/.DS_Store
Outdated
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.
restore file
src/services/fileStorage/.DS_Store
Outdated
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.
restore file
@@ -227,7 +228,7 @@ export class LessonCopyService { | |||
copiedContent.push(linkContent); | |||
copiedContentStatus.push(embeddedTaskStatus); | |||
} | |||
if (element.component === ComponentType.NEXBOARD && nexboardEnabled) { | |||
if (element.component === ComponentType.NEXBOARD && nexboardEnabled && copyNexboardEnabled) { |
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.
the previous solution with status NOT_DOING was better
so, why not applying the condition just at that level instead of skiping the who thing?
the correct behaviour is to inform the client that nexboard is not copied (on purpose).
93914c2
to
5db47fd
Compare
Description
This is the server implementation to exclude nexboard when user try to copy and share courses.
Links to Tickets or other pull requests
https://ticketsystem.dbildungscloud.de/browse/BC-5119
hpi-schul-cloud/schulcloud-client#3313
Changes
Datasecurity
Deployment
New Repos, NPM pakages or vendor scripts
Approval for review
generate-client:server
was executed in vue frontend and changes were tested and put in a PR with the same branch name.