Do not show HalfDays if corresponding PresenceType is inactive #169
Conversation
There was a problem hiding this comment.
Würde die Checks positiv aufbauen (siehe Kommentare), sieht ansonsten gut aus.
Mit GymHofwyl konnte ich beim Edit den Fall wo der Halbtag aktiv ist nicht testen, da dieser NeedsConfirmation: false hat und rausgefiltert wird. Hast du dies einmal in echt testen können, ob der Halbtag angezeigt wird?
| (c) => c !== Category.HalfDay | ||
| ); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Ich würde versuchen dies positiv zu machen, d.h. die Halbtag-Option hinzufügen, wenn aktiv. Sonst poppt sie kurz auf und verschwindet wieder, je nachdem wie lange der Request dauer, nicht? Ev. könntest du gleich den ganzen categories Array dynamisch setzen...
| this.absenceTypes$ | ||
| .pipe( | ||
| map((types) => | ||
| types.filter((t) => isHalfDay(t, this.settings) && t.Active) |
There was a problem hiding this comment.
Micro-Improvement: hier könntest du ein Boolean(types.find((t) => isHalfDay(t, this.settings))?.Active) machen und gleich auf einen boolean mappen. Das hat auch zum Vorteil, dass das Iterieren abgebrochen wir, sobald der Halfday gefunden wird.
| this.presenceTypes$ | ||
| .pipe( | ||
| map((types) => | ||
| types.filter((t) => isHalfDay(t, this.settings) && t.Active) |
There was a problem hiding this comment.
Hier das gleiche wie beim Edit.
| if (types.length === 0) { | ||
| this.columns = this.columns.filter((c) => c.key !== 'TotalHalfDays'); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Auch hier, würde ich versuchen dies positiv zu machen.
caebr
force-pushed
the
feature/125-presence-type-half-day
branch
from
b7c70bd to
cca117e
Compare
Nein bisher nicht. Ich habe im Ticket nachgefragt, wie ich das testen kann. |
| ); | ||
| .subscribe((activeHalfDay) => { | ||
| if (activeHalfDay) { | ||
| this.categories.push(Category.HalfDay); |
There was a problem hiding this comment.
Hier müsstest du noch die Reihenfolge beachten (so kommt der Halbtag am Schluss).
There was a problem hiding this comment.
Alternativ könntest du auch einen availableCategories Array mit allen Kategorien machen und einen activeCategories für den du setzt, indem du die availableCategories filterst...
There was a problem hiding this comment.
Würdest du dies dann für die die Columns nicht auch gleich so machen (available und dann filtern nach active)? Damit es einheitlich ist? Aber dann sind wir zurück bei den negativen Checks :)
| this.columns = this.columns.filter((c) => c.key !== 'TotalHalfDays'); | ||
| .subscribe((activeHalfDay) => { | ||
| if (activeHalfDay) { | ||
| this.columns.push({ key: 'TotalHalfDays', label: 'halfday' }); |
There was a problem hiding this comment.
Du kannst den ReadonlyArray behalten und immutable arbeiten: this.columns = [...this.columns, { key: 'TotalHalfDays', label: 'halfday' }]
caebr
force-pushed
the
feature/125-presence-type-half-day
branch
from
48cc457 to
82def49
Compare
Testen mit test2@test.ch, BsBZI