-
Notifications
You must be signed in to change notification settings - Fork 16
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
Custom create parameters in room type settings #574
Custom create parameters in room type settings #574
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #574 +/- ##
=============================================
+ Coverage 95.82% 95.86% +0.03%
- Complexity 1288 1295 +7
=============================================
Files 198 198
Lines 4412 4430 +18
=============================================
+ Hits 4228 4247 +19
+ Misses 184 183 -1 ☔ View full report in Codecov by Sentry. |
d69cb37
to
b1e7c72
Compare
If the room type settings are changed in the meantime, the room type is always 'reset'. Only refreshing the list solves this issue. 2023-09-19.17-06-27.mp4 |
Looks like this bug already exists |
I would suggest to resolve this issue in a separate pull request. I continue trying to fix the listed errors in the PHP checks. |
@dennis531 I'll have a look |
@dennis531 I fixed the issue |
dd2b06c
to
159e115
Compare
@SamuelWei what does this need need now to be merged? |
@tibroc Frontend and backend tests; if I havn't missed it, the custom params are shown in the UI once the type is changed, but they should also be applied on creating a new room with this type; to improve the UX it might also be good to show the user what settings will be changed, giving the user to option to accept oder dismiss the changes. |
@dennis531 @tibroc We are currently working on something similar in https://github.com/THM-Health/PILOS/tree/695-restructure-room-settings-page As soon as this is finished, we will have an other look at this PR and see how we can integrate this |
a77e32b
to
589cfd0
Compare
Fixes #559
Type (Highlight the corresponding type)
Checklist
Changes
Other information