-
Notifications
You must be signed in to change notification settings - Fork 14
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-7796 - Room Details, Create, Update, Delete #3386
Conversation
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 could not add much in this review. The code works.
There are several pieces missing and marked with TODOs (e.g. No content) which are known and out of scope.
API are implemented. Room create/delete/edit is working.
Breadcrumbs are working.
Needs more testing from QA.
Some e2e tests are failing and sonarcloud wants some metrics to be met.
I'm approving the code as it's functional and don't have any heavy defects.
Only problem I might found is that the rooms tab is not hidden behind a feature flag. But that was introduced before. Must double check.
const roomApi = RoomApiFactory(undefined, "/v3", $axios); | ||
const isLoading = ref(true); | ||
|
||
const roomData = ref<RoomCreateParams>({ |
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.
Maybe you can clarify why we would use a ref instead of reactive?
https://blog.logrocket.com/reactivity-vue-3-composition-api-ref-reactive/
In my limited understanding it seems that we are using an object and this is the typical use case for using reactive.
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.
Refs are generally easier to use and work just as well. Afaik a while back when I was wondering about this as well a lot of blogs recommend to just use refs for everything as it's simpler and more versatile.
This reverts commit 9c72ba6.
…-schul-cloud/nuxt-client into BC-7796-room-details" This reverts commit e539a85, reversing changes made to 81d4be9.
….com/hpi-schul-cloud/nuxt-client into BC-7796-room-details"" This reverts commit afb2030.
Quality Gate failedFailed conditions |
Short Description
User story
As a teacher, I want to have a room page for all my room content (boards etc), to have a starting point for my lessons or whatever the room is for.
Acceptance criteria
Links to Ticket and related Pull-Requests
BC-7796
Changes
Data-security
Deployment
New Repos, NPM packages or vendor scripts
Screenshots of UI changes
Checklist before merging