Implement Vue substitution frontend

magicfelix commented

2023-08-08 16:55:32 +02:00

(Migrated from edugit.org)

When a user has the required permission(*), lessons in the timetable should have a "manage" or "substituion" or something like that, button. Clicking on it should open a dialog (?), in which subject, teachers and room can be altered and an option to mark the lesson as cancelled and to write a comment are provided.

*: See rules.py in Chronos. There are things like chronos.edit_substitution_rule that can be used and (if necessary) be adjusted.

When a user has the required permission(*), lessons in the timetable should have a "manage" or "substituion" or something like that, button. Clicking on it should open a dialog (?), in which subject, teachers and room can be altered and an option to mark the lesson as cancelled and to write a comment are provided. *: See rules.py in Chronos. There are things like `chronos.edit_substitution_rule` that can be used and (if necessary) be adjusted.

magicfelix commented

2023-08-08 16:55:33 +02:00

(Migrated from edugit.org)

assigned to @permcu

magicfelix commented

2023-08-08 16:57:31 +02:00

(Migrated from edugit.org)

changed the description

magicfelix commented

2023-08-08 17:08:39 +02:00

(Migrated from edugit.org)

changed the description

magicfelix commented

2023-08-16 20:12:05 +02:00

(Migrated from edugit.org)

changed the description

permcu commented

2023-08-23 17:56:59 +02:00

(Migrated from edugit.org)

created branch 211-implement-vue-substitution-frontend to address this issue

created branch [`211-implement-vue-substitution-frontend`](/AlekSIS/official/AlekSIS-App-Chronos/-/compare/master...211-implement-vue-substitution-frontend) to address this issue

permcu commented

2023-08-23 17:58:00 +02:00

(Migrated from edugit.org)

mentioned in merge request !614

permcu commented

2023-08-24 13:07:29 +02:00

(Migrated from edugit.org)

@magicfelix: The rules you mentioned are from Lesrooster. I think we should stay in Chronos. Could you update them please.

magicfelix commented

2023-08-24 13:20:59 +02:00

(Migrated from edugit.org)

changed the description

magicfelix commented

2023-08-24 13:21:21 +02:00

(Migrated from edugit.org)

Updated

permcu commented

2023-09-02 12:57:35 +02:00

(Migrated from edugit.org)

The substitution frontend (via amend LessonEvents) works now. Backend
and LessonEvent handling is finished.

This leaves us with a few more TODOs:

TODO: PRIO (@yuha ) Currently selectedEvent is only updated
in backend & frontend has to wait for
poll. Instead selectedEvent should
update immediatly on any changes in
substitution frontend.

permcu: This is not solved at my end. Try changing a lesson
        to cancelled and back again. This change takes a
        while to show. Not even a reload solves it. @yuha 
        Please try the specific test. Changes should be
        shown when the spinner stops spinning.
permcu: I looked into this some more. Updating does not work
        reliably at my end. But one important issue so far
        is that a substitution shows immediately in the
        calendar but the details view is not updated while
        staying open. This needs to change.

TODO: (@hansegucker ) Display of cancelled lessons is
broken. Only cancelled, all other
amends seem to work. Cancelled lesson
is displayed without subject. Pls
fix.
TODO: @hansegucker Timezones
TODO: Beautify the DialogObjectForm:
- [?] Display in one column
- & use a bigger comment field. (@yuha)
TODO: PRIO: Only show amend button if chronos.edit_substitution_rule permission.
TODO: PRIO: Either substituted or cancelled in frontend!
TODO: Add cancel (shortcut) button.
TODO: Add remove substitution button.
TODO: Show error messages
permcu: I do not understand which error not shown
MAYBE: Select only teachers instead of all persons (Use same solution in cursus as well!)

FIXME/TODO: Fix default value for autocomplete for subject, teachers & rooms
See 5161437cf9 commit msg for more context.
Default value is id, as was before, does not semm to work work.

            Look at edit-object. This is UI only since edit-object is used in query.

            Changing :value-comparator to ="(a, b) => a.id === b.id" 
            and using objects seems to but selects the
            wrong (first) default value and makes the field
            non editable.

MAYBE: Move substitution component from LessonDetails to own component?

@yuha & @hansegucker do think we could ship the subfrontend? It
is my understanding that there is someone waiting on it.

Imho if PRIO is done we could ship it if needed.

I could do the button (s?) at the end of this week if still
necessary.

The substitution frontend (via amend LessonEvents) works now. Backend and LessonEvent handling is finished. This leaves us with a few more TODOs: - [x] TODO: PRIO (@yuha ) Currently selectedEvent is only updated in backend & frontend has to wait for poll. Instead selectedEvent should update immediatly on any changes in substitution frontend. permcu: This is not solved at my end. Try changing a lesson to cancelled and back again. This change takes a while to show. Not even a reload solves it. @yuha Please try the specific test. Changes should be shown when the spinner stops spinning. permcu: I looked into this some more. Updating does not work reliably at my end. But one important issue so far is that a substitution shows immediately in the calendar but the details view is not updated while staying open. This needs to change. - [x] TODO: (@hansegucker ) Display of cancelled lessons is broken. Only cancelled, all other amends seem to work. Cancelled lesson is displayed without subject. Pls fix. - [x] TODO: @hansegucker Timezones - [x] TODO: Beautify the DialogObjectForm: - [?] Display in one column - [X] & use a bigger comment field. (@yuha) - [x] TODO: PRIO: Only show amend button if chronos.edit_substitution_rule permission. - [x] TODO: PRIO: Either substituted or cancelled in frontend! - [ ] TODO: Add cancel (shortcut) button. - [x] TODO: Add remove substitution button. - [x] TODO: Show error messages permcu: I do not understand which error not shown - [ ] MAYBE: Select only teachers instead of all persons (Use same solution in cursus as well!) - [x] FIXME/TODO: Fix default value for autocomplete for subject, teachers & rooms See 5161437cf9f84255eb87dd749f68b17066a13b28 commit msg for more context. Default value is id, as was before, does not semm to work work. Look at edit-object. This is UI only since edit-object is used in query. Changing :value-comparator to ="(a, b) => a.id === b.id" and using objects seems to but selects the wrong (first) default value and makes the field non editable. - [ ] MAYBE: Move substitution component from LessonDetails to own component? @yuha & @hansegucker do think we could ship the subfrontend? It is my understanding that there is someone waiting on it. Imho if PRIO is done we could ship it if needed. I could do the button (s?) at the end of this week if still necessary.

yuha commented

2023-09-02 13:20:10 +02:00

(Migrated from edugit.org)

Auto-updating after save of amended lesson is now working

yuha commented

2023-09-02 13:26:32 +02:00

(Migrated from edugit.org)

The comment field is also bigger now; but is beautifying the DialogObjectForm in order to make its column count configurable really necessary? Please note that in case the device width is rather small (like on mobile phones) the column count is automatically set to 1 and the DIalogObjectForm is expanded to cover the full screen, so it should already be guaranteed that the form is not too cramped.

hansegucker commented

2023-09-02 16:59:02 +02:00

(Migrated from edugit.org)

@permcu We also need an option to delete a substitution.

permcu commented

2023-09-04 18:05:38 +02:00

(Migrated from edugit.org)

It is not necessary. My rationale was to visually link the cancelled toggle and the comment textfield since I think they belong together. The two column layout puts them in two different columns.

Also I just happen to like the one column look for this Form:

But MAYBE, as @hansegucker mentioned in the chat, we should not do a one off solution here, but instead think more generally about DialogObjectForm usecases and appearance.

It is not necessary. My rationale was to visually link the cancelled toggle and the comment textfield since I think they belong together. The two column layout puts them in two different columns. Also I just happen to like the one column look for this Form: ![image](/uploads/eabf0b5c92a785de5c0fa96fd6dd59e2/image.png) But MAYBE, as @hansegucker mentioned in the chat, we should not do a one off solution here, but instead think more generally about DialogObjectForm usecases and appearance.

permcu commented

2023-09-04 23:24:22 +02:00

(Migrated from edugit.org)

I thought just leaving an empty substitution would be OK in this case. To delete a substitution you just remove the fields in the substitution menu. This does not expose to the user that there is a substitution LessonEvent.

Other solution could be: Use a delete mutation and add a del/rem button if a LessonEvent is amended.

Do you strongly prefer the second option? Or did you have something else in mind?

I thought just leaving an empty substitution would be OK in this case. To delete a substitution you just remove the fields in the substitution menu. This does not expose to the user that there is a substitution LessonEvent. Other solution could be: Use a delete mutation and add a del/rem button if a LessonEvent is amended. Do you strongly prefer the second option? Or did you have something else in mind?

permcu commented

2023-09-04 23:40:03 +02:00

(Migrated from edugit.org)

Not for me. See above.

permcu commented

2023-09-05 13:00:56 +02:00

(Migrated from edugit.org)

Regarding a substitution is either substituted or cancelled in frontend!

On @hansegucker initiative I changed the frontend to only allow subject, teachers, rooms substitution if the lesson is not cancelled.

But currently if there was aready a new room it is still passed as default (edit-item) to the amendLessonMutation. This needs the TODO above to be shown to the user.

But more importantly we have to decide how the data model should look like. Or if @hansegucker has already something in mind. Please explain it to me.

Originally I thought substitutions would stack. = for every change you add a new substitution lessonevent. But apparantly there should only ever be one substitution per lesson. This means the first substitution creates the amend event which is thereafter mutated. Is this correct?

This means susbstituted information is lost each time a substitution changes.

Imho it is OK for a cancelled lesson to have a new room the same way it is okay for it to have an old room.

Regarding a substitution is either substituted or cancelled in frontend! On @hansegucker initiative I changed the frontend to only allow subject, teachers, rooms substitution if the lesson is not cancelled. But currently if there was aready a new room it is still passed as default (edit-item) to the amendLessonMutation. This needs the TODO above to be shown to the user. But more importantly we have to decide how the data model should look like. Or if @hansegucker has already something in mind. Please explain it to me. Originally I thought substitutions would stack. = for every change you add a new substitution lessonevent. But apparantly there should only ever be one substitution per lesson. This means the first substitution creates the amend event which is thereafter mutated. Is this correct? This means susbstituted information is lost each time a substitution changes. Imho it is OK for a cancelled lesson to have a new room the same way it is okay for it to have an old room.

hansegucker commented

2023-09-05 17:32:54 +02:00

(Migrated from edugit.org)

Originally I thought substitutions would stack. = for every change you add a new substitution lessonevent. But apparantly there should only ever be one substitution per lesson. This means the first substitution creates the amend event which is thereafter mutated. Is this correct?

This is correct.

This means susbstituted information is lost each time a substitution changes.

How?

Imho it is OK for a cancelled lesson to have a new room the same way it is okay for it to have an old room.

Yeah, well, but if you say: This event is the new status. There is a new room, but the lesson isn't actually happening – that's unnecessarily confusing in my opinion.

> Originally I thought substitutions would stack. = for every change you add a new substitution lessonevent. But apparantly there should only ever be one substitution per lesson. This means the first substitution creates the amend event which is thereafter mutated. Is this correct? This is correct. > This means susbstituted information is lost each time a substitution changes. How? > Imho it is OK for a cancelled lesson to have a new room the same way it is okay for it to have an old room. Yeah, well, but if you say: This event is the new status. There is a new room, but the lesson isn't actually happening – that's unnecessarily confusing in my opinion.

hansegucker commented

2023-09-05 17:33:25 +02:00

(Migrated from edugit.org)

Second option is preferred.

permcu commented

2023-09-10 20:58:37 +02:00

(Migrated from edugit.org)

This means susbstituted information is lost each time a substitution changes.

How?

First substitute with teacher A, then substitute again with teacher B (maybe A got ill or something)
Now the information that teacher A was the first substitute is lost. It was mutated. But maybe this does not matter much? I would prefer for a substitution to leave a record and not vanish.

Imho it is OK for a cancelled lesson to have a new room the same way it is okay for it to have an old room.

Yeah, well, but if you say: This event is the new status. There is a new room, but the lesson isn't actually happening – that's unnecessarily confusing in my opinion.

The confusion is avoided if cancel overrides. That means a cancelled lesson does not need to highlight that it was amended otherwise. But if the user clicks on the cancelled lesson for more information I think amened info should still be shown. It would be more confusing otherwise. Ie was the original lesson cancelled or the substituted.

> > This means susbstituted information is lost each time a substitution changes. > How? First substitute with teacher A, then substitute again with teacher B (maybe A got ill or something) Now the information that teacher A was the first substitute is lost. It was mutated. But maybe this does not matter much? I would prefer for a substitution to leave a record and not vanish. > > Imho it is OK for a cancelled lesson to have a new room the same way it is okay for it to have an old room. > Yeah, well, but if you say: This event is the new status. There is a new room, but the lesson isn't actually happening – that's unnecessarily confusing in my opinion. The confusion is avoided if cancel overrides. That means a cancelled lesson does not need to highlight that it was amended otherwise. But if the user clicks on the cancelled lesson for more information I think amened info should still be shown. It would be more confusing otherwise. Ie was the original lesson cancelled or the substituted.

permcu commented

2023-09-10 21:22:17 +02:00

(Migrated from edugit.org)

@hansegucker cancelling does remove the class from the calendar & removing cancel does not put it back. Also even an empty amend lessonEvent does leave the calendar event orange. I am beginning to understand why you want a delete substitution option. Since only this clears the UI.

@hansegucker cancelling does remove the class from the calendar & removing cancel does not put it back. Also even an empty amend lessonEvent does leave the calendar event orange. I am beginning to understand why you want a delete substitution option. Since only this clears the UI. ![image](/uploads/8a139b38cd4760821cb27df630985095/image.png)

hansegucker commented

2023-09-13 21:07:31 +02:00

(Migrated from edugit.org)

.

yuha commented

2023-09-20 00:14:07 +02:00

(Migrated from edugit.org)

Should be fixed now – the details are now updated correctly (using https://edugit.org/AlekSIS/official/AlekSIS-Core/-/merge_requests/1318)

permcu commented

2023-09-20 18:10:55 +02:00

(Migrated from edugit.org)

Nice :) I already thought I'd have to do it myself;)

permcu commented

2023-10-08 16:49:32 +02:00

(Migrated from edugit.org)

created branch 211-implement-vue-substitution-frontend-2 to address this issue

created branch [`211-implement-vue-substitution-frontend-2`](/AlekSIS/official/AlekSIS-App-Chronos/-/compare/master...211-implement-vue-substitution-frontend-2) to address this issue

hansegucker commented

2024-03-28 17:47:38 +01:00

(Migrated from edugit.org)

mentioned in commit 210217945f

mentioned in commit 210217945f7de01db3fd54e1b57b04ac59b0b806

hansegucker (Migrated from edugit.org) closed this issue

2024-03-28 17:47:38 +01:00

nik commented

2026-02-18 12:48:27 +01:00

(Migrated from edugit.org)

unassigned @permcu