Details des Tickets
Beschreibung,Kommentare und Anhänge
| assigned to @gabrielg |
Sascha Immig |
2022-09-21T16:06:08.022Z |
| statt
`{{ __(':supervisor', ['supervisor' => $supervisor->salutated_name]) }}`
sollte man einfach`
{{ $supervisor->salutated_name }}`
verwenden |
Gregor Gabriel |
2022-09-22T12:49:26.026Z |
| statt
```php
$supervisedCategories = [];
foreach ($supervisor->supervisedCategories()->pluck('title') as $category) {
$supervisedCategories[] = $category;
}
```
ist folgendes viel einfacher:
`$supervisedCategories = $supervisor->supervisedCategories()->pluck('title')->toArray();` |
Gregor Gabriel |
2022-09-22T13:36:58.968Z |
| `$supervisedCategories` wird zwar in der Funktion befüllt,
aber es wird nicht genutzt und auch nicht zurückgegeben?
Warum ist das im Code? |
Gregor Gabriel |
2022-09-22T13:44:43.482Z |
| `$supervisedOrganizationUnits` wird zwar in der Funktion befüllt,
aber es wird nicht genutzt und auch nicht zurückgegeben?
Warum ist das im Code? |
Gregor Gabriel |
2022-09-22T13:44:50.857Z |
| Frage: ist es explizit gewollt, dass nur die beiden unten aufgeführten Rollen genutzt werden dürfen.
D.h. wenn ich eine der beiden Rollen kopiere und noch Rechte dazu tu, dann ist diese Kopie dennoch nicht geeignet? |
Gregor Gabriel |
2022-09-22T14:35:40.396Z |
| ich glaube, es ging um den expliziten Ausschluss des Client-Admins. Ich bin mir grade nicht sicher, ob das final korrekt ist...
Lasst und das nochmal genauer betrachten und eruieren. Generell ist hier die Überprüfung einer Permission sinnvoller, als direkt auf die Rolle zu gehen... |
Sascha Immig |
2022-09-23T09:04:43.219Z |
| @gabrielg
die untenstehende Methode `supervisorsOfCategories()` prüft nicht, ob die Kategorie evtl. gelöscht wurde.
Wenn die Kategorie gelöscht würde, sollte diese doch automatisch überall entfernt werden und hier erst gar nicht auftauchen.
@gabrielg
die hier vorgeschlagene Implementierung tut die und liest sich einfacher
Was genau ist hiermit gemeint? Welche vorgeschlagene Implementierung? |
Markus Gerlach |
2022-09-23T10:42:52.551Z |
| changed this line in version 2 of the diff |
Markus Gerlach |
2022-09-23T10:46:09.940Z |
| changed this line in version 2 of the diff |
Markus Gerlach |
2022-09-23T10:46:10.082Z |
| changed this line in version 2 of the diff |
Markus Gerlach |
2022-09-23T10:46:10.175Z |
| changed this line in version 2 of the diff |
Markus Gerlach |
2022-09-23T10:46:10.269Z |
added 1 commit
- 17b789ae - Added queries if(! empty($organizationUnit->parent_id)) to training function Supervisors()
Compare with previous version |
Markus Gerlach |
2022-09-23T10:46:10.695Z |
| siehe Frage oben |
Gregor Gabriel |
2022-09-23T14:49:00.121Z |
| wurde eine Kategorie gelöscht, so ist diese lediglich als gelöscht markiert.
Das erkennt diese Implementierung von `supervisorsOfCategories()` |
Gregor Gabriel |
2022-09-23T15:04:45.858Z |
| in meinem Kommentar oben findet sich ein Vorschlag für eine alternative Implementierung |
Gregor Gabriel |
2022-09-23T15:10:09.476Z |
| die untenstehende (im Code nach supervisorsOfCategories() suchen) Methode `supervisorsOfCategories()` prüft nicht, ob die Kategorie evtl. gelöscht wurde.
Alternativer Vorschlag:
```php
/**
* returns a collection of all supervisors from categories
* this training belongs to
*
* @return instance of Illuminate\Database\Eloquent\Builder for supervisors
*/
public function supervisorsOfCategories()
{
return User::active()
->capableOfBeingSupervisor()
->whereHas('supervisedCategories', function ($query) {
$query->whereHas('trainings', function ($query) {
$query->where('trainings.id', $this->id);
});
});
}
```
die hier vorgeschlagene Implementierung tut die und liest sich einfacher |
Gregor Gabriel |
2022-09-23T15:12:18.233Z |
| Das sind die:
Trainingsbeauftragten für die aktuelle Kategorie in den übergeordneten Abteilungen (nachfolgend) |
Markus Gerlach |
2022-09-26T06:34:47.102Z |
| Habe meine Beschreibung noch etwas konkretisiert |
Gregor Gabriel |
2022-09-26T06:49:10.077Z |
| changed this line in version 3 of the diff |
Markus Gerlach |
2022-09-26T07:04:22.962Z |
added 1 commit
- 2a50d067 - LL-1200 now also checks if the category has been deleted.
Compare with previous version |
Markus Gerlach |
2022-09-26T07:04:23.292Z |
| ja genau
```
public function supervisorsRolesOfUser()
{
return User::active()
->role([User::ROLE_CONTROLLER, User::ROLE_TRAINING_MANAGEMENT])
->where('client_id', auth()->user()->client_id);
}
```
Alternativ könnte man hier auch über Rechte gehen per
```
public function supervisorsRolesOfUser()
{
$capableOfBeingSupervisor =
User::active()
->capableOfBeingSupervisor()
->where('client_id', auth()->user()->client_id);
...
return $capableOfBeingSupervisorWithoutRoleClientAdmins;
}
```
Frage:
Wollen / Entfernen wir hier dann auch die User die z.B. beide Rollen ClientAdmin und Controller haben? |
Markus Gerlach |
2022-09-26T07:08:09.364Z |
| heisst das, dass ein user der nur die Rolle `OrganizationUnit::POSITION_SUPERVISOR` hat, an dieser Stelle keinen Abteilungsvorgesetzten hat?
Konkret:
```
+ Deutschland (Markus: Positon::Supervisor, Role::CanSuperviseTraining)
-+ Bayern (Ronnie: Positon::Supervisor, Role::CanSuperviseTraining)
-+ München (Gregor: Positon::Employee)
```
Hier bekommt Gregor den Training-Supervisor Ronnie aber Ronnie nicht automatisch Markus, da Ronnie kein Employee ist
Ist das mit Sascha abgesprochen? |
Gregor Gabriel |
2022-09-26T07:43:46.019Z |
| ok dann sollte es klar sein. Auch Vorgesetzt können natürlich Vorgesetzte haben.
Demnach kann die Zeile einefach raus:
`->where('role', OrganizationUnit::POSITION_EMPLOYEE)`
daduch muss noch eingebunden werden das die eigene id noch ausgeschlossen werden muss. |
Markus Gerlach |
2022-09-26T08:12:22.130Z |
| @immigs bitte prüfen |
Markus Gerlach |
2022-09-26T08:14:18.970Z |
| @immigs bitte prüfen |
Markus Gerlach |
2022-09-26T08:15:05.908Z |
| `capableOfBeingSupervisor()` prüft ja auf Permissions::BE_TRAINING_SUPERVISOR. Das umfasst ja alle User, bei denen die Box im Datenblatt angehakt ist. Das umfasst mehr Leute als die obigen beiden Rollen...
Da kommt es jetzt auf dne Kontext an, ob das der korrekte Ersatz wäre... |
Sascha Immig |
2022-09-28T07:34:31.915Z |
| Ja, bitte die Vorgesetzten von Vorgesetzten berücksichtigen.
Bei Gregors Beispiel: Markus muss der in Punkt 5 ausgewählte Vorgesetzte für Ronnie sein.
Zusatz: wäre Ronnie kein Vorgesetzter (also Bayern ohne Führungskraft), wäre Markus der ausgewählte Berater für Gregor |
Sascha Immig |
2022-09-28T07:37:41.493Z |
| ich glaube, das Thema gestaltet sich etwas komplizierter als gedacht:
Nochmal zur Klarstellung:
1. Trainingsbeauftragter (`Permissions::BE_TRAINING_SUPERVISOR`?) für die aktuelle Kategorie in der eigenen Abteilung oder (nachfolgend, d.h. es wurde kein Beauftragter gefunden) in übergeordneten Abteilungen
2. Sonstige Trainingsbeauftragten in der der eigenen Abteilung oder (nachfolgend) in übergeordneten Abteilungen
-> m.E. müssten wir also alle Trainingsbeauftragten holen und prüfen, ob die in der Hierarchie von der eigenen Abteilung des Fragestellers hinauf zum Root eingeordnet sind und ob die Kategorie übereinstimmt.
Stimmt die Kategorie nicht, traversieren wir weiter hoch. Stimmt die Kategorie, dann `break`, weil wir den korrekten Trainingsbeauftragten gefunden haben. Landen wir im Root, ohne einen Beauftragten für die Kategorie zu haben, nehmen wir den ersten Trainingsbeauftragten einer anderen Kategorie, der am nächsten zu eigenen Abteilung des Fragestellers in der Hierarchie zu finden ist.
3. Alle weiteren Trainingsbeauftragten für die Kategorie (entspricht der bisherigen 1)
-> finden wir keinen Trainingsbeauftragten für die Kategorie in der direkten Hierarchie, nehmen wir *alle* Trainingsbeauftragten für die Kategorie im Mandanten
4. Alle sonstigen Trainingsbeauftragten (entspricht der bisherigen 2)
-> haben wir keinen Trainingsbeauftragten für die korrekte Kategorie , nehmen wir einfach *alle* Trainingsbeauftragten im Mandanten, unabhängig von Ihrer Hierarchie
5. Vorgesetzte in der der eigenen Abteilung oder (nachfolgend) in übergeordneten Abteilungen
6. Client-Admins (entspricht der bisherigen 3)
-> An dieser Stelle ist ausgeschlossen, dass es einen Trainingsbeauftragten irgendwo im Mandanten gibt. Dann gehen wir den kompletten Baum von der eigenen Abteilung des Fragestellers hoch bis zum Root. der erste User, der ein Vorgesetzter ist, wird gewählt. (entweder ich finde so einen Vorgesetzten in der gleichen Abteilung oder irgendwo auf dem Weg zum Root.) Sind in einer Abteilung 2 Personen Vorgesetzte, sollten auch beide ausgewählt werden.Ganz am Ende - also wenn es gar keine Trainingsbeauftragten oder Vorgesetzten gibt (z.B. bei Basic Kunden) - sollen die globalen Client-Admins benachrichtigt werden. Hier wäre jetzt noch zu klären, ob wir nur die Admins benachrichtigen oder auch die Controller/ Trainingsmanager... |
Sascha Immig |
2022-09-28T07:52:32.548Z |
| Das sind die:
Trainingsbeauftragten für die aktuelle Kategorie in der eigenen Abteilung
Das ist ein Teil der Aufgabe. Detailliert zu finden in der PDF des Tickets http://jira.learningsystem.de/secure/attachment/16669/Trainingsr%C3%BCckfragen%20OE.pdf
Kurzform aus der PDF:
1. Trainingsbeauftragter für die aktuelle Kategorie in der eigenen Abteilung oder (nachfolgend) in
übergeordneten Abteilungen
2. Sonstige Trainingsbeauftragten in der der eigenen Abteilung oder (nachfolgend) in
übergeordneten Abteilungen
3. Alle weiteren Trainingsbeauftragten für die Kategorie (entspricht der bisherigen 1), FYI kann auch der Client-Admin mit betreuten Kategorien sein
4. Alle sonstigen Trainingsbeauftragten (entspricht der bisherigen 2)
5. Vorgesetzte in der der eigenen Abteilung oder (nachfolgend) in übergeordneten Abteilungen
6. Client-Admins (entspricht der bisherigen 3) |
Markus Gerlach |
2022-09-28T14:07:10.935Z |
| changed this line in version 4 of the diff |
Markus Gerlach |
2022-09-28T14:36:53.192Z |
| changed this line in version 4 of the diff |
Markus Gerlach |
2022-09-28T14:36:53.506Z |
| changed this line in version 4 of the diff |
Markus Gerlach |
2022-09-28T14:36:53.664Z |
| resolved all discussions |
Markus Gerlach |
2022-09-28T14:36:53.735Z |
added 1 commit
- 4da1ae39 - LL-1224 include the superiors of superiors and exclude current user
Compare with previous version |
Markus Gerlach |
2022-09-28T14:36:53.967Z |
| resolved all discussions |
Markus Gerlach |
2022-09-28T14:37:04.437Z |
| genau diese Permission erhalten die Client Admins, Controller und Trainingsmanager im Client Model.
Demnach müssten oben im Code nur noch die Clients entfernt werden.
Bleibt die Frage:
Wollen / Entfernen wir hier dann auch die User die z.B. beide Rollen ClientAdmin und Controller haben?
@immigs bitte prüfen |
Markus Gerlach |
2022-09-28T14:39:23.668Z |
| siehe oben |
Markus Gerlach |
2022-09-28T14:39:41.037Z |
| resolved all discussions |
Markus Gerlach |
2022-09-28T14:39:42.786Z |
added 1 commit
- d6a8dbde - composer require twig/twig:2.15.3
Compare with previous version |
Markus Gerlach |
2022-09-29T07:44:44.480Z |
added 1 commit
- 0847c470 - added additional condition category for part 2
Compare with previous version |
Markus Gerlach |
2022-09-29T09:37:35.538Z |
added 195 commits
- 0847c470...52123eb8 - 194 commits from branch
develop - 048417b1 - Merge branch 'develop' into feature/LL-1200-training-queries-with-departments
Compare with previous version |
Markus Gerlach |
2022-10-14T12:50:56.249Z |
| changed this line in version 14 of the diff |
Markus Gerlach |
2022-12-08T15:08:38.474Z |
| Title |
Id |
Author |
Created at |
Message |
| indention [skip-ci] |
36da2b82c7c20a2d08701e7b571f8d2b63cc0f9d |
TUV\immigs |
2023-01-10T10:55:55.000Z |
indention [skip-ci]
|
| Merge branch 'develop' into feature/LL-1200-training-queries-with-departments |
371f5c46a7a6daa56ceed6e29cb766dc5692b32f |
TUV\immigs |
2023-01-09T13:03:41.000Z |
Merge branch 'develop' into feature/LL-1200-training-queries-with-departments
|
| optimized code for better readability and exchanged elseif for if. |
38d80302593779a3dcf975e8e3d49136ec5ad4c8 |
TUV\immigs |
2023-01-06T10:56:19.000Z |
optimized code for better readability and exchanged elseif for if.
|
| LL-1200 Reintegrated ROLE_TRAINING_REPRESENTATIVE of the version from 2022-22-09 |
43000a78d16ee40347a13cce1af3cc6d7d1b058e |
gerlachm |
2022-12-02T08:20:23.000Z |
LL-1200 Reintegrated ROLE_TRAINING_REPRESENTATIVE of the version from 2022-22-09
|
| added scope for User class to filter out only users, that supervise a specific training |
6019bbdb076c91569cddef63d2c1dbeddb3b43be |
TUV\gabrielg |
2022-12-08T12:33:29.000Z |
added scope for User class to filter out only users, that supervise a specific training
|
| added TODO comments |
624ca7d44f1b0a6c986af215d85fef0d4b9e0196 |
TUV\immigs |
2023-01-06T12:23:46.000Z |
added TODO comments
|
| more refactoring |
7eaebcab9ae355194d8e7eaf1ccf0889b6e24c25 |
TUV\immigs |
2023-01-09T13:08:38.000Z |
more refactoring
|
| use whereDoesntHave('roles', function/* name=admin */) instead of… |
7fcf89b7fa537b9551bcf02289187436ff997852 |
TUV\gabrielg |
2023-01-10T15:31:18.000Z |
use whereDoesntHave('roles', function/* name=admin */) instead of whereHas('roles', function/*name!=admin*/)
|
| Merge branch 'feature/LL-1200-training-queries-with-departments' of… |
81da8f55d3d1e618b8aa80009e4847a937887dfa |
TUV\gabrielg |
2022-12-08T12:36:12.000Z |
Merge branch 'feature/LL-1200-training-queries-with-departments' of https://gitlab.learningsystem.de/lms/seventeen into feature/LL-1200-training-queries-with-departments
# Conflicts:
# app/Models/User.php
|
| LL-1200-training-queries-with-departments |
91ce51a791be9d1ac777cce4ce09c374f3dcdbb0 |
gerlachm |
2022-12-08T15:08:33.000Z |
LL-1200-training-queries-with-departments
Optimizations andfixed typeError fot sApp\Models\User::supervisedTrainings()
|
| resolved todos |
af5960fa4c63857af66cb0f64f95d150ab83498d |
TUV\immigs |
2023-01-09T13:01:50.000Z |
resolved todos
|
| optimized texts: |
b010281e7f7323efccdffb67ec5e1e0cc773cbd1 |
TUV\immigs |
2023-01-09T17:15:17.000Z |
optimized texts:
- moved to new translation structure.
- used gender neutral speech in de.
- replaced list with plain text (as it was before).
- removed categories due too much complexity when n supervisors with m categires meet a training with q categories.
|
| LL-1200 removed exception of client |
bc09461344f5bcf909a8378a9e642d62f7c1078e |
gerlachm |
2022-12-12T11:30:54.000Z |
LL-1200 removed exception of client
|
| LL-1200 optimization and the role "demo catalog" is set as default |
bc859033d7e2a8b411aba0eecb6ba9a06b43966b |
gerlachm |
2022-12-12T10:54:10.000Z |
LL-1200 optimization and the role "demo catalog" is set as default
|
| Merge branch 'develop' into feature/LL-1200-training-queries-with-departments |
c8637bb3d5b14c63fa0f274f529dc4f8e3a19374 |
gerlachm |
2022-12-01T13:35:31.000Z |
Merge branch 'develop' into feature/LL-1200-training-queries-with-departments
# Conflicts:
# app/Models/User.php
|
| added some refactoring due to phpStorm |
daa8bfa9d2e3812c863a324bf8bb98e5748905e9 |
TUV\immigs |
2023-01-09T13:02:27.000Z |
added some refactoring due to phpStorm
|
| fist case (supervisor for category) restricted to active users only |
e382645422facae13590a6d70dc93c0e8768f8a5 |
TUV\immigs |
2023-01-10T14:34:07.000Z |
fist case (supervisor for category) restricted to active users only
|
| optimized comments / documentation |
edac496df979fce56d3ea4bb61808ff1f87041cd |
TUV\immigs |
2023-01-06T09:49:33.000Z |
optimized comments / documentation
|
| Merge branch 'develop' into feature/LL-1200-training-queries-with-departments |
f1e48dbbbcc7a7b836c0c354e62bb3196708efdd |
gerlachm |
2022-12-08T08:53:41.000Z |
Merge branch 'develop' into feature/LL-1200-training-queries-with-departments
# Conflicts:
# app/Models/Training.php
# app/Models/User.php
|
| Merge branch 'feature/LL-1200-training-queries-with-departments' of… |
fd9615519d15ba1880e55ab43f37ce4940f6ef36 |
TUV\gabrielg |
2023-01-10T15:31:38.000Z |
Merge branch 'feature/LL-1200-training-queries-with-departments' of https://gitlab.learningsystem.de/lms/seventeen into feature/LL-1200-training-queries-with-departments
|