-
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-8168 - Implementing video conferences in FE and remaining issues #5420
Conversation
apps/server/src/modules/board-context/board-context-api-helper.service.ts
Outdated
Show resolved
Hide resolved
apps/server/src/modules/board-context/board-context-api-helper.service.ts
Show resolved
Hide resolved
apps/server/src/modules/board/service/board-node-authorizable.service.ts
Show resolved
Hide resolved
|
||
return board; | ||
return { board, features }; |
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.
this seems to be the simplest solution to just add the features in the initial response.
we should not really follow strict rest as we are building an interactive web app and not a static directory
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.
Do you mean adding it to ColumnBoard
? That would mean altering the domain object.
apps/server/src/modules/board-context/board-context-api-helper.service.ts
Outdated
Show resolved
Hide resolved
apps/server/src/modules/board-context/board-context-api-helper.service.ts
Outdated
Show resolved
Hide resolved
|
||
if ( | ||
this.isVideoConferenceEnabledForConfig() && | ||
(await this.isVideoConferenceEnabledForSchool(course.school.id)) && |
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.
silly observation, but await inside if statement shoudl be avoided
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.
Will keep an eye on that. Should work for now I think.
}; | ||
|
||
describe('when video conference is enabled for course', () => { | ||
describe('and video conference is enabled for school and config', () => { |
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.
too many nested describe is also a bit confusing. Better having them separated with their own setup
Quality Gate passedIssues Measures |
Description
Implementing video conferences in FE and remaining issues
Links to Tickets or other pull requests
BC-8168
hpi-schul-cloud/nuxt-client#3480
hpi-schul-cloud/dof_app_deploy#1086
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.