HNNW/core
Watch
28
2
Fork
You've already forked core
 0
Code Issues Pull requests 8 Releases Activity Actions

team-5/integrate-symfony-ux #28

Open
caylen-ramazan wants to merge 33 commits from team-5/integrate-symfony-ux into develop
pull from: team-5/integrate-symfony-ux
merge into: HNNW:develop
Conversation 6 Commits 33 Files changed 28 +847 -644
caylen-ramazan commented 2026-09-29 08:00:40 +00:00
Member
Copy link

er zijn voor 3 pages live components toegevoegd edit-participants, invite-participants en projects in deze pages is de main functionaliteit in javascript vervangen voor symfony UX live components en de javascript files en referenties zijn verwijderd.
Er zijn 2 symfony twig components toegevoegd een alert pop up en een profile card.
in commit 3c62fa48b3 heb ik de import './styles/app.css'; uit assets/app.js verwijderd omdat dat er voor zorgde dat live components niet werkte met toestemming van de PO.
Paginering in projects view is bewust weg gehaald
In projectService word gebruik gemaakt van statusFixtures die wel in de develop branch staat.

er zijn voor 3 pages live components toegevoegd edit-participants, invite-participants en projects in deze pages is de main functionaliteit in javascript vervangen voor symfony UX live components en de javascript files en referenties zijn verwijderd. Er zijn 2 symfony twig components toegevoegd een alert pop up en een profile card. in commit 3c62fa48b3 heb ik de import './styles/app.css'; uit assets/app.js verwijderd omdat dat er voor zorgde dat live components niet werkte met toestemming van de PO. Paginering in projects view is bewust weg gehaald In projectService word gebruik gemaakt van statusFixtures die wel in de develop branch staat.
Reviewed-on: #27
rileydeman requested changes 2026-10-02 09:52:16 +00:00
Dismissed
rileydeman left a comment
Copy link

Goed werk om de drie pagina's om te zetten naar Live Components, er is veel losse JavaScript weg en de Alert/ProfileCard-components zijn herbruikbaar. Het weghalen van import './styles/app.css' klopt ook: dat bestand bestaat niet in develop.

Dit moet nog aangepast worden:

Moet opgelost worden

  1. ProjectsView::getProjects() gebruikt $this->profileRepository->findOneBy([]). Dat haalt een willekeurig profiel op, dus iedereen ziet de projecten van een willekeurige gebruiker. Gebruik het ingelogde profiel (bijv. via de CurrentProfileProvider).
  2. Autorisatie in InviteParticipantsSearch: projectId is een LiveProp(writable: true) en inviteSelectedProfiles() controleert niet of de gebruiker dit project mag beheren. Via een aangepaste request kan iemand dan deelnemers toevoegen aan elk project. Maak projectId niet writable en doe in de LiveAction een permissiecheck (bijv. denyAccessUnlessGranted met de ProjectVoter).
  3. Hetzelfde geldt voor ModifyParticipantsTable: de participant-ID's en role-ID's in rolesByParticipant worden niet gecontroleerd. Zo kun je rollen wijzigen van deelnemers in andere projecten. Controleer dat participant én rol bij $this->projectId horen.
  4. InviteParticipantsService zet de status met findOneBy([]) ("random entity for now"). Gebruik de juiste status. Haal het project ook één keer op buiten de loop, controleer of het bestaat, en doe de flush() na de loop in plaats van per profiel.
  5. ProfileRepository::searchExcluding(): heeft een project nog geen deelnemers, dan is $excludedProfileIds leeg. Doctrine maakt van NOT IN (:ids) met een lege lijst NOT IN (NULL), en dan komt er niets terug. Bij een nieuw project kun je dus niemand vinden. Voeg die andWhere alleen toe als de lijst niet leeg is.

Verbeterpunten
6. In het projectoverzicht zijn de paginering en de keuze voor items per pagina verdwenen. Is dat bewust?
7. De link href="org/projects/{{ project.slug }}" is relatief. Gebruik path().
8. Er staan ongebruikte imports en constructor-dependencies in: Security, LiveArg en ProjectParticipantRepository in ProjectsView, en NotFoundHttpException in ProfileRepository.
9. Het oude formulier in modify-participants.html.twig staat er nog als comment. Haal het weg.
10. De component-templates (Alert, ProfileCard) hebben een {% block stylesheets %} met een <link>. Die wordt bij elk gebruik van het component opnieuw in de body gerenderd. Laad de CSS liever via de pagina of de layout.
11. ProfileCard.html.twig gebruikt {{ type }}, maar dat is geen property van het component.
12. {% if this.projects == [] %} kan {% if this.projects is empty %} worden.

Let op: PR #15 (team 6) wijzigt modify-participants.html.twig ook, op een andere manier. Stem af wie wat doet.

Goed werk om de drie pagina's om te zetten naar Live Components, er is veel losse JavaScript weg en de Alert/ProfileCard-components zijn herbruikbaar. Het weghalen van `import './styles/app.css'` klopt ook: dat bestand bestaat niet in develop. Dit moet nog aangepast worden: **Moet opgelost worden** 1. `ProjectsView::getProjects()` gebruikt `$this->profileRepository->findOneBy([])`. Dat haalt een willekeurig profiel op, dus iedereen ziet de projecten van een willekeurige gebruiker. Gebruik het ingelogde profiel (bijv. via de `CurrentProfileProvider`). 2. Autorisatie in `InviteParticipantsSearch`: `projectId` is een `LiveProp(writable: true)` en `inviteSelectedProfiles()` controleert niet of de gebruiker dit project mag beheren. Via een aangepaste request kan iemand dan deelnemers toevoegen aan elk project. Maak `projectId` niet writable en doe in de LiveAction een permissiecheck (bijv. `denyAccessUnlessGranted` met de ProjectVoter). 3. Hetzelfde geldt voor `ModifyParticipantsTable`: de participant-ID's en role-ID's in `rolesByParticipant` worden niet gecontroleerd. Zo kun je rollen wijzigen van deelnemers in andere projecten. Controleer dat participant én rol bij `$this->projectId` horen. 4. `InviteParticipantsService` zet de status met `findOneBy([])` ("random entity for now"). Gebruik de juiste status. Haal het project ook één keer op buiten de loop, controleer of het bestaat, en doe de `flush()` na de loop in plaats van per profiel. 5. `ProfileRepository::searchExcluding()`: heeft een project nog geen deelnemers, dan is `$excludedProfileIds` leeg. Doctrine maakt van `NOT IN (:ids)` met een lege lijst `NOT IN (NULL)`, en dan komt er niets terug. Bij een nieuw project kun je dus niemand vinden. Voeg die `andWhere` alleen toe als de lijst niet leeg is. **Verbeterpunten** 6. In het projectoverzicht zijn de paginering en de keuze voor items per pagina verdwenen. Is dat bewust? 7. De link `href="org/projects/{{ project.slug }}"` is relatief. Gebruik `path()`. 8. Er staan ongebruikte imports en constructor-dependencies in: `Security`, `LiveArg` en `ProjectParticipantRepository` in ProjectsView, en `NotFoundHttpException` in ProfileRepository. 9. Het oude formulier in `modify-participants.html.twig` staat er nog als comment. Haal het weg. 10. De component-templates (Alert, ProfileCard) hebben een `{% block stylesheets %}` met een `<link>`. Die wordt bij elk gebruik van het component opnieuw in de body gerenderd. Laad de CSS liever via de pagina of de layout. 11. `ProfileCard.html.twig` gebruikt `{{ type }}`, maar dat is geen property van het component. 12. `{% if this.projects == [] %}` kan `{% if this.projects is empty %}` worden. Let op: PR #15 (team 6) wijzigt `modify-participants.html.twig` ook, op een andere manier. Stem af wie wat doet.
Reviewed-on: #36
rileydeman requested changes 2026-10-12 12:55:19 +00:00
Dismissed
rileydeman left a comment
Copy link

Goed werk met de feedback: projectId is niet meer writable, er is een ProjectVoter met een permissions-mask, ModifyParticipantsTable controleert nu of deelnemer en rol bij het project horen, searchExcluding() werkt met een lege lijst, het project wordt één keer opgehaald met een enkele flush(), het oude formulier in comments is weg, en de link gebruikt nu path(). Er zit alleen nog een blokkerende fout in de autorisatie.

Moet opgelost worden

  1. ProjectVoter is abstract class. Symfony registreert geen abstracte classes als service, dus de voter doet niet mee. Daardoor geeft isGranted('PROJECT_INVITE' / 'PROJECT_EDIT') altijd false, en kan niemand deelnemers uitnodigen of rollen wijzigen. Haal abstract weg (eventueel final).
  2. ProjectsView::getProjects() geeft het resultaat van CurrentProfileProvider::getProfile() door aan findByProfileOnQuery(string $query, Profile $profile). getProfile() kan null geven: niet ingelogd, of ingelogd via de huidige sessie-login, want de provider op develop kijkt alleen naar Symfony Security. Dan krijg je een TypeError. Handel null af (lege lijst teruggeven). PR #12 (team 3) maakt de provider geschikt voor de sessie-login, dus stem af wie eerst merget.
  3. isActiveParticipant() doet nu findActiveParticipantsByProjectAndProfile(...) !== null. Die methode geeft altijd een array terug, nooit null, dus iedereen is "actieve deelnemer". Daardoor kan iedereen zich inschrijven op taken (WorkPackageTaskEnrollmentService). Gebruik !== [] of findLatestActiveParticipantByProjectAndProfile(...) !== null.
  4. (punt 4 vorige review) InviteParticipantsService zet de status nog steeds met findOneBy([]) ("random entity for now"). Haal de status op met scope ProjectParticipant (zie StatusFixtures). Controleer ook of $project bestaat.
  5. ProjectParticipantUpdateRoleHandler::handle() roept $role->getProject() aan zonder te checken of $role bestaat. Een onbekend of leeg role-ID geeft een fatal error. Voeg if (!$role) { continue; } toe.

Verbeterpunten
6. use SebastianBergmann\CodeCoverage\StaticAnalysis\Visibility; in ProjectVoter is een per ongeluk toegevoegde import uit PHPUnit. Haal die weg, net als de andere ongebruikte imports: Security, LiveArg, ProjectParticipantRepository en ProfileRepository in ProjectsView.
7. getProject(): project (kleine letter) in InviteParticipantsSearch en ModifyParticipantsTable geeft een TypeError als het project niet bestaat. Schrijf Project en handel null af.
8. $errorMessage is writable: true. Dat is niet nodig, en zo kan de client de melding zelf zetten. Maak hem niet-writable. De meldingen zijn nu Engels ("You are not allowed to invite profile…"), terwijl de rest van de site Nederlands is.
9. findActiveParticipant() is hernoemd naar findActiveParticipantsByProjectAndProfile(). PR #12 (team 3) implementeert juist findActiveParticipant(), dus dat geeft een conflict. Stem af welke naam blijft.
10. De projectkaarten in het org-panel linken nu naar main.projects.detail (de publieke projectpagina) in plaats van naar de beheerpagina in het org-panel. Is dat bewust? De paginering en de keuze voor items per pagina (punt 6 vorige review) zijn nog steeds weg.

Let op: PR #46 (zelfde auteur) gebruikt deze ProjectVoter ook en kan pas werken als deze PR gemerged is.

Goed werk met de feedback: `projectId` is niet meer writable, er is een `ProjectVoter` met een permissions-mask, `ModifyParticipantsTable` controleert nu of deelnemer en rol bij het project horen, `searchExcluding()` werkt met een lege lijst, het project wordt één keer opgehaald met een enkele `flush()`, het oude formulier in comments is weg, en de link gebruikt nu `path()`. Er zit alleen nog een blokkerende fout in de autorisatie. **Moet opgelost worden** 1. `ProjectVoter` is `abstract class`. Symfony registreert geen abstracte classes als service, dus de voter doet niet mee. Daardoor geeft `isGranted('PROJECT_INVITE' / 'PROJECT_EDIT')` altijd `false`, en kan niemand deelnemers uitnodigen of rollen wijzigen. Haal `abstract` weg (eventueel `final`). 2. `ProjectsView::getProjects()` geeft het resultaat van `CurrentProfileProvider::getProfile()` door aan `findByProfileOnQuery(string $query, Profile $profile)`. `getProfile()` kan `null` geven: niet ingelogd, of ingelogd via de huidige sessie-login, want de provider op develop kijkt alleen naar Symfony Security. Dan krijg je een TypeError. Handel `null` af (lege lijst teruggeven). PR #12 (team 3) maakt de provider geschikt voor de sessie-login, dus stem af wie eerst merget. 3. `isActiveParticipant()` doet nu `findActiveParticipantsByProjectAndProfile(...) !== null`. Die methode geeft altijd een array terug, nooit `null`, dus iedereen is "actieve deelnemer". Daardoor kan iedereen zich inschrijven op taken (`WorkPackageTaskEnrollmentService`). Gebruik `!== []` of `findLatestActiveParticipantByProjectAndProfile(...) !== null`. 4. (punt 4 vorige review) `InviteParticipantsService` zet de status nog steeds met `findOneBy([])` ("random entity for now"). Haal de status op met scope `ProjectParticipant` (zie `StatusFixtures`). Controleer ook of `$project` bestaat. 5. `ProjectParticipantUpdateRoleHandler::handle()` roept `$role->getProject()` aan zonder te checken of `$role` bestaat. Een onbekend of leeg role-ID geeft een fatal error. Voeg `if (!$role) { continue; }` toe. **Verbeterpunten** 6. `use SebastianBergmann\CodeCoverage\StaticAnalysis\Visibility;` in `ProjectVoter` is een per ongeluk toegevoegde import uit PHPUnit. Haal die weg, net als de andere ongebruikte imports: `Security`, `LiveArg`, `ProjectParticipantRepository` en `ProfileRepository` in `ProjectsView`. 7. `getProject(): project` (kleine letter) in `InviteParticipantsSearch` en `ModifyParticipantsTable` geeft een TypeError als het project niet bestaat. Schrijf `Project` en handel `null` af. 8. `$errorMessage` is `writable: true`. Dat is niet nodig, en zo kan de client de melding zelf zetten. Maak hem niet-writable. De meldingen zijn nu Engels ("You are not allowed to invite profile…"), terwijl de rest van de site Nederlands is. 9. `findActiveParticipant()` is hernoemd naar `findActiveParticipantsByProjectAndProfile()`. PR #12 (team 3) implementeert juist `findActiveParticipant()`, dus dat geeft een conflict. Stem af welke naam blijft. 10. De projectkaarten in het org-panel linken nu naar `main.projects.detail` (de publieke projectpagina) in plaats van naar de beheerpagina in het org-panel. Is dat bewust? De paginering en de keuze voor items per pagina (punt 6 vorige review) zijn nog steeds weg. Let op: PR #46 (zelfde auteur) gebruikt deze `ProjectVoter` ook en kan pas werken als deze PR gemerged is.
patrick requested changes 2026-10-13 11:36:43 +00:00
patrick left a comment
Copy link

Bij participants/invite en modify krijg je een exception wanneer je weizigingen probeert door te voeren. Dit komt, omdat er aan de alert in InviteParticipantsSearcht en ModifyParticipantsTable 'this.errorMessage' wordt gegeven als content, terwijl alert geen errorMessage property heeft. Om dit op te lossen verwijder de "this" uit die regel, hierdoor wordt de errorMessage property van de parent gebruikt.

Bij participants/invite en modify krijg je een exception wanneer je weizigingen probeert door te voeren. Dit komt, omdat er aan de alert in InviteParticipantsSearcht en ModifyParticipantsTable 'this.errorMessage' wordt gegeven als content, terwijl alert geen errorMessage property heeft. Om dit op te lossen verwijder de "this" uit die regel, hierdoor wordt de errorMessage property van de parent gebruikt.
rileydeman requested changes 2026-10-14 10:47:20 +00:00
Dismissed
rileydeman left a comment
Copy link

Goed werk, alle blokkerende punten uit de vorige review zijn opgelost. ProjectVoter is nu final en doet dus mee, getProjects() geeft een lege lijst zonder profiel, isActiveParticipant() checkt op !== [], de status komt uit scope ProjectParticipant en een onbekende rol wordt overgeslagen. Ook het punt van Patrick over {{ errorMessage }} is verwerkt.

(punt 1 t/m 5 vorige review) opgelost

Dit moet nog aangepast worden:

Moet opgelost worden

  1. (punt 7 vorige review) InviteParticipantsSearch::getProject() heeft return-type Project, maar geeft null terug als het project niet bestaat. Dat geeft een TypeError, dus de errorMessage die je daar zet wordt nooit getoond. Maak het return-type ?Project en handel null af waar je getProject() gebruikt. ModifyParticipantsTable::getProject() heeft hetzelfde probleem (en project met kleine letter).
  2. (punt 8 vorige review) $errorMessage in ModifyParticipantsTable is nog #[LiveProp(writable: true)]. Dan kan de browser de foutmelding zelf aanpassen. Haal writable: true weg, net als in InviteParticipantsSearch.
  3. InviteParticipantsService importeert App\DataFixtures\Common\StatusFixtures. Productiecode mag niet naar fixtures verwijzen (fixtures zijn er in productie niet). Haal de import weg.

Verbeterpunten
4. (punt 6 vorige review) Ongebruikte imports/dependencies in ProjectsView: Security, LiveArg, ProfileRepository en ProjectParticipantRepository.
5. findActiveParticipant() in ProjectParticipantRepository wordt nergens gebruikt en geeft een exception bij meer dan één participant-rij. Is hij voor PR #12? Stem dan af wie hem houdt, anders weghalen.
6. De meldingen zijn Engels en de melding in ModifyParticipantsTable::updateParticipants() heeft het over "invite" terwijl het om rollen wijzigen gaat.
7. (punt 10 vorige review) De paginering in ProjectsView is weg. Is dat bewust?

Let op: PR #12 (team 3) voegt dezelfde findActiveParticipant() toe. PR #46 (zelfde auteur) hangt van deze PR af.

Goed werk, alle blokkerende punten uit de vorige review zijn opgelost. `ProjectVoter` is nu `final` en doet dus mee, `getProjects()` geeft een lege lijst zonder profiel, `isActiveParticipant()` checkt op `!== []`, de status komt uit scope `ProjectParticipant` en een onbekende rol wordt overgeslagen. Ook het punt van Patrick over `{{ errorMessage }}` is verwerkt. (punt 1 t/m 5 vorige review) opgelost Dit moet nog aangepast worden: **Moet opgelost worden** 1. (punt 7 vorige review) `InviteParticipantsSearch::getProject()` heeft return-type `Project`, maar geeft `null` terug als het project niet bestaat. Dat geeft een TypeError, dus de `errorMessage` die je daar zet wordt nooit getoond. Maak het return-type `?Project` en handel `null` af waar je `getProject()` gebruikt. `ModifyParticipantsTable::getProject()` heeft hetzelfde probleem (en `project` met kleine letter). 2. (punt 8 vorige review) `$errorMessage` in `ModifyParticipantsTable` is nog `#[LiveProp(writable: true)]`. Dan kan de browser de foutmelding zelf aanpassen. Haal `writable: true` weg, net als in `InviteParticipantsSearch`. 3. `InviteParticipantsService` importeert `App\DataFixtures\Common\StatusFixtures`. Productiecode mag niet naar fixtures verwijzen (fixtures zijn er in productie niet). Haal de import weg. **Verbeterpunten** 4. (punt 6 vorige review) Ongebruikte imports/dependencies in `ProjectsView`: `Security`, `LiveArg`, `ProfileRepository` en `ProjectParticipantRepository`. 5. `findActiveParticipant()` in `ProjectParticipantRepository` wordt nergens gebruikt en geeft een exception bij meer dan één participant-rij. Is hij voor PR #12? Stem dan af wie hem houdt, anders weghalen. 6. De meldingen zijn Engels en de melding in `ModifyParticipantsTable::updateParticipants()` heeft het over "invite" terwijl het om rollen wijzigen gaat. 7. (punt 10 vorige review) De paginering in `ProjectsView` is weg. Is dat bewust? Let op: PR #12 (team 3) voegt dezelfde `findActiveParticipant()` toe. PR #46 (zelfde auteur) hangt van deze PR af.
rileydeman requested changes 2026-10-14 16:56:56 +00:00
Dismissed
rileydeman left a comment
Copy link

Netjes opgepakt. getProject() geeft nu in beide components ?Project terug en de acties stoppen met een foutmelding als het project niet bestaat, errorMessage in ModifyParticipantsTable is niet meer writable, de fixtures-import is weg, findActiveParticipant() is verwijderd en de melding bij rollen wijzigen klopt nu. Ook importmap('app') staat nu buiten het javascript-block, zodat Stimulus op alle org-pagina's laadt.

(punt 1, 2, 3, 5 en 6 vorige review) opgelost

Dit moet nog aangepast worden:

Moet opgelost worden

  1. (punt 7 vorige review) De paginering op de projectenpagina is nog steeds weg: ProjectsView::getProjects() haalt alle projecten in één keer op en de "Items per page"/"Page x of y"-controls uit projects.html.twig zijn verwijderd. Bij veel projecten wordt de pagina lang en traag. Zet de paginering terug (bijv. een #[LiveProp(writable: true)] int $page met limit/offset in findByProfileOnQuery()), of laat in de PR weten dat het bewust weg is en dat dit is afgestemd.
  2. (punt 4 vorige review, deels) ProjectsView krijgt nog steeds ProfileRepository en ProjectParticipantRepository in de constructor, maar gebruikt ze nergens. Dat suggereert dat de component profielen zelf ophaalt (zoals eerst met findOneBy([])). Haal beide dependencies en hun imports weg; Security en LiveArg zijn al weg.

Verbeterpunten
3. In ProjectsView.html.twig staat Public page:{{ project.slug }} en Edit page:{{ project.slug }} zonder spatie na de dubbele punt.
4. Kleine opmaak: return[], if ($project === null){ en isGranted('PROJECT_INVITE',$project) missen spaties. Laat eventueel PHP-CS-Fixer erover lopen.

Let op: PR #46 (zelfde auteur) hangt van deze PR af en wijzigt ook org.base.html.twig (importmap). Rebase #46 op develop nadat deze PR gemerged is.

Netjes opgepakt. `getProject()` geeft nu in beide components `?Project` terug en de acties stoppen met een foutmelding als het project niet bestaat, `errorMessage` in `ModifyParticipantsTable` is niet meer writable, de fixtures-import is weg, `findActiveParticipant()` is verwijderd en de melding bij rollen wijzigen klopt nu. Ook `importmap('app')` staat nu buiten het `javascript`-block, zodat Stimulus op alle org-pagina's laadt. (punt 1, 2, 3, 5 en 6 vorige review) opgelost Dit moet nog aangepast worden: **Moet opgelost worden** 1. (punt 7 vorige review) De paginering op de projectenpagina is nog steeds weg: `ProjectsView::getProjects()` haalt alle projecten in één keer op en de "Items per page"/"Page x of y"-controls uit `projects.html.twig` zijn verwijderd. Bij veel projecten wordt de pagina lang en traag. Zet de paginering terug (bijv. een `#[LiveProp(writable: true)] int $page` met limit/offset in `findByProfileOnQuery()`), of laat in de PR weten dat het bewust weg is en dat dit is afgestemd. 2. (punt 4 vorige review, deels) `ProjectsView` krijgt nog steeds `ProfileRepository` en `ProjectParticipantRepository` in de constructor, maar gebruikt ze nergens. Dat suggereert dat de component profielen zelf ophaalt (zoals eerst met `findOneBy([])`). Haal beide dependencies en hun imports weg; `Security` en `LiveArg` zijn al weg. **Verbeterpunten** 3. In `ProjectsView.html.twig` staat `Public page:{{ project.slug }}` en `Edit page:{{ project.slug }}` zonder spatie na de dubbele punt. 4. Kleine opmaak: `return[]`, `if ($project === null){` en `isGranted('PROJECT_INVITE',$project)` missen spaties. Laat eventueel PHP-CS-Fixer erover lopen. Let op: PR #46 (zelfde auteur) hangt van deze PR af en wijzigt ook `org.base.html.twig` (importmap). Rebase #46 op develop nadat deze PR gemerged is.
rileydeman requested changes 2026-10-19 08:38:19 +00:00
rileydeman left a comment
Copy link

Goed dat je de ongebruikte ProfileRepository en ProjectParticipantRepository uit ProjectsView hebt gehaald, en dat er nu een limiet op het aantal projecten zit. De spaties in "Public page:" / "Edit page:", return [] en isGranted('PROJECT_INVITE', $project) zijn ook netjes.

Vorige review

  • (punt 1) half: er is een "Max Results"-veld bijgekomen, maar dat doet nog niets (zie punt 1 hieronder).
  • (punt 2) opgelost.
  • (punt 3) opgelost.
  • (punt 4) grotendeels opgelost, if ($project === null){ in InviteParticipantsSearch mist nog een spatie.

Dit moet nog aangepast worden:

Moet opgelost worden

  1. ProjectsView::getProjects() roept nog steeds findByProfileOnQuery($this->query, $profile) aan, zonder $this->maxResults. Daardoor gebruikt de repository altijd de standaard 30 en verandert er niets als je het "Max Results"-veld aanpast. Geef de waarde mee: findByProfileOnQuery($this->query, $profile, $this->maxResults). Begrens hem ook, want maxResults is een writable LiveProp en de gebruiker kan dus elk getal sturen (0, negatief of heel groot). Bijvoorbeeld max(1, min($this->maxResults, 100)) in getProjects() en min="1" max="100" op het input-veld. Zet maxResults in reset() ook terug naar 30.
  2. In findByProfileOnQuery() staat setMaxResults() zonder orderBy(). PostgreSQL geeft dan geen vaste volgorde terug, dus welke 30 projecten je ziet kan per keer verschillen. Voeg een sortering toe, bijv. ->orderBy('project.title', 'ASC').
  3. (punt 1 vorige review) Met alleen een limiet kan een gebruiker met meer projecten dan het maximum de rest niet meer bereiken, en hij ziet ook niet dat er projecten ontbreken. Zet de paginering terug (een #[LiveProp(writable: true)] int $page met setFirstResult(($page - 1) * $maxResults) en vorige/volgende-knoppen), of laat in de PR weten dat een limiet in plaats van paginering bewust is en dat dit is afgestemd.

Verbeterpunten
4. Geef $maxResults in findByProfileOnQuery() een type en spaties: int $maxResults = 30. Ook findByProfileOnQuery($this->query,$profile) en if ($project === null){ missen een spatie.

Let op: PR #46 (zelfde auteur) hangt van deze PR af. Ik heb getest dat #46 na deze PR zonder conflicten merget. Met de huidige develop geeft de proefmerge van deze PR ook geen conflicten.

Goed dat je de ongebruikte `ProfileRepository` en `ProjectParticipantRepository` uit `ProjectsView` hebt gehaald, en dat er nu een limiet op het aantal projecten zit. De spaties in "Public page:" / "Edit page:", `return []` en `isGranted('PROJECT_INVITE', $project)` zijn ook netjes. **Vorige review** - (punt 1) half: er is een "Max Results"-veld bijgekomen, maar dat doet nog niets (zie punt 1 hieronder). - (punt 2) opgelost. - (punt 3) opgelost. - (punt 4) grotendeels opgelost, `if ($project === null){` in `InviteParticipantsSearch` mist nog een spatie. Dit moet nog aangepast worden: **Moet opgelost worden** 1. `ProjectsView::getProjects()` roept nog steeds `findByProfileOnQuery($this->query, $profile)` aan, zonder `$this->maxResults`. Daardoor gebruikt de repository altijd de standaard `30` en verandert er niets als je het "Max Results"-veld aanpast. Geef de waarde mee: `findByProfileOnQuery($this->query, $profile, $this->maxResults)`. Begrens hem ook, want `maxResults` is een writable LiveProp en de gebruiker kan dus elk getal sturen (0, negatief of heel groot). Bijvoorbeeld `max(1, min($this->maxResults, 100))` in `getProjects()` en `min="1" max="100"` op het input-veld. Zet `maxResults` in `reset()` ook terug naar 30. 2. In `findByProfileOnQuery()` staat `setMaxResults()` zonder `orderBy()`. PostgreSQL geeft dan geen vaste volgorde terug, dus welke 30 projecten je ziet kan per keer verschillen. Voeg een sortering toe, bijv. `->orderBy('project.title', 'ASC')`. 3. (punt 1 vorige review) Met alleen een limiet kan een gebruiker met meer projecten dan het maximum de rest niet meer bereiken, en hij ziet ook niet dat er projecten ontbreken. Zet de paginering terug (een `#[LiveProp(writable: true)] int $page` met `setFirstResult(($page - 1) * $maxResults)` en vorige/volgende-knoppen), of laat in de PR weten dat een limiet in plaats van paginering bewust is en dat dit is afgestemd. **Verbeterpunten** 4. Geef `$maxResults` in `findByProfileOnQuery()` een type en spaties: `int $maxResults = 30`. Ook `findByProfileOnQuery($this->query,$profile)` en `if ($project === null){` missen een spatie. Let op: PR #46 (zelfde auteur) hangt van deze PR af. Ik heb getest dat #46 na deze PR zonder conflicten merget. Met de huidige develop geeft de proefmerge van deze PR ook geen conflicten.
This pull request doesn't have enough approvals yet. 0 of 2 approvals granted.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin team-5/integrate-symfony-ux:team-5/integrate-symfony-ux
git switch team-5/integrate-symfony-ux
Sign in to join this conversation.
Reviewers
No reviewers
patrick
rileydeman
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
HNNW/core!28
No description provided.