-
Notifications
You must be signed in to change notification settings - Fork 1
User profile and PDF export of memberships #42
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
base: main
Are you sure you want to change the base?
Conversation
app/Livewire/ChangePassword.php
Outdated
|
|
||
| public function mount($username) | ||
| { | ||
| if ($username == auth()->user()->username || auth()->user()->can('superadmin', User::class)) { |
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.
=== statt ==
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.
in Livewire kann man auch $this->authorize() verwenden, dann liefert es aber auch direkt ein Permission denied wenns schief geht. Ich glaube ich fände es schöner eine Policy für PW Changes zu haben und auf die hier zu authorizen. Dann ist wer darf was schön an einem Platz (bei den Policies). Kannst du beim User mit rein nehmen.
| { | ||
| $username = Auth::user()->username; | ||
| $user = User::findOrFailByUsername($username); | ||
| if ($username == auth()->user()->username || auth()->user()->can('superadmin', User::class)) { |
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.
s.o.
| public string $fullName; | ||
|
|
||
| public function mount() | ||
| public $currentUsername; |
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.
Vorsicht! Public probs können via client api frei verändert werden, auch wenn du da kein Input feld hin machst. Macht das hier Probleme? Evtl das [#Url] oder [#Locked] Prop verwenden stattdessen
| } | ||
| } | ||
|
|
||
| public function getMemberships(string $username, bool $onlyActive) |
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.
public Methoden können bei livewire auch vom client aus aufgerufen werden. Wenn du das nicht machst (wie hier?) solltest du die private machen
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.
Moderator:innen einer Studischaft haben keinen Zugriff auf den Profil-Bereich. Damit Moderator:innen trotzdem die Möglichkeit haben, die Tätigkeitsübersicht zu exportieren und es möglichst wenig Code-Dopplung gibt, greift auch der Members-Controller auf diese Funktion zu.
app/Livewire/Profile/Memberships.php
Outdated
| $memberships = []; | ||
| foreach ($roleMemberships as $row) { | ||
| $role = Role::findOrFail('cn=' . $row->role_cn . ',' . $row->committee_dn); | ||
| array_push($memberships, [ |
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.
Note: If you use array_push() to add one element to the array, it's better to use $array[] = because in that way there is no overhead of calling a function.
https://www.php.net/manual/en/function.array-push.php
Ich finde $memberships[] = [...]; auch leichter verständlich was passiert :)
app/Livewire/Profile/Memberships.php
Outdated
|
|
||
| return response()->streamDownload(function () use ($pdf) { | ||
| echo $pdf->stream(); | ||
| }, 'memberships-' . $this->currentUsername . '.pdf');; |
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.
memberships im File Name hier auch übersetzen? @DieMichii was ist ein Guter Dateiname für eine Gremienbescheinigung die wie im Chat aussieht?
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.
p.s. man kann auch an die Übersetzung parameter (wie den Namen) übergeben. Das wäre hier vmtl. sinnig.
| unset($this->deleteMemberName, $this->deleteMemberUsername); | ||
| } | ||
|
|
||
| public function exportPdf($username) |
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.
Sehe ich das Richtig, das es einmal das PDF pro Person und das andere mal das PDF pro Rolle / Realm ist?
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.
Falls du dich auf den Zugriff aus unterschiedlichen Komponenten auf diese Funktion beziehst, dann ist der einzige Unterschied, dass im Profil kein Realm mitgegeben werden kann, da die Nutzer nicht nur einem einzigen Realm zugeordnet sein können. Beim Export über die Mitglieder-Liste eines Realms kann der aktuelle Realm für den PDF-Export mitgegeben werden, sodass auch die Studischaft im Dokument angezeigt werden kann.
| <x-table.cell>{{ $realm_member->cn[0] }}</x-table.cell> | ||
| <x-table.cell> | ||
| @if (auth()->user()->can('superadmin', \App\Ldap\User::class)) | ||
| <a wire:navigate href="{{ route('profile', ['username' => $realm_member->uid[0]]) }}"> |
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.
ich weiß das es schon so war, aber hier ist es vllt besser das native getFirstAttribute von LdapRecord zu verwenden. Der aktuelle Code würde explodieren falls uid bspw kein Array wäre
| <html> | ||
| <head> | ||
| <meta charset="utf-8" /> | ||
| <style> |
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.
das war nicht mit Tailwind möglich?
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.
Das würde bedeuten, dass wir Tailwind auch in den PDF-Export einbinden müssten. Das ist für die geringen Anforderungen eigentlich nicht unbedingt nötig. Ob die Integration von Tailwind an der Stelle funktioniert, weiß ich nicht genau. Hübscher wird es bei der geringe Komplexität mit normalem CSS aber aus meiner Sicht nicht werden.
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.
DOMPDF unterstützt nur CSS 2.1 und ein wenig CSS 3 (siehe https://github.com/dompdf/dompdf).
* Save relation between groups and roles in database and sync group members to LDAP by script * Script to move the roles of all groups from LDAP to the database
No description provided.