feat(SCHUL-611): Notificatie Allegro status update by RubenSibon · Pull Request #380 · Amsterdam/schulddossier

RubenSibon · 2026-01-22T13:47:54Z

No description provided.

* feat: enable proxy usage * build(release): file w/ tag * build(Make): re-add node versions * build(xdebug): client host * feat: missing files :-) * feat: make import schuldeisers command working from local * build(make): add command to enter pod * refactor(allegro): move duplicate code to helper function * feat(allegro): implement proxy in more services * refactor(allegro): make it more DRY * refactor(allegrohelper): make code more consistent * build(docker): pull and restart policies * chore(cleanup): unused imports and spacing --------- Co-authored-by: jmassink <j.massink@amsterdam.nl>

…ossier into release/SCHUL_sprint-7

* build(release): file w/ tag * build(Make): re-add node versions * build(xdebug): client host * build(deps): composer upgrade * build(deps): npm update * build(deps): bump webpack-dev-server from 4.15.2 to removed in the npm_and_yarn group across 1 directory (#372) * build(deps): bump webpack-dev-server Bumps the npm_and_yarn group with 1 update in the / directory: [webpack-dev-server](https://github.com/webpack/webpack-dev-server). Removes `webpack-dev-server` --- updated-dependencies: - dependency-name: webpack-dev-server dependency-version: dependency-type: indirect dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] <support@github.com> * build(deps): upgrade symfony webpack encore And related pacakges. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ruben Sibon <r.sibon@amsterdam.nl> * build(deps): upgrade jspdf majorly --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…ossier into release/SCHUL_sprint-7

style(editorconfig): update linelength

…ossier into feat/SCHUL-611_notificatie-allegro-status-update

src/Command/SyncDossierWithAllegroCommand.php

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

feat(dossierDasboard.html.twig): don't show export button to normal users

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

tvanoort85

So far lijkt dit wel ok te zijn :-) heb het niet getest maar codewise heb ik geen mededelingen anders dan die die al gedaan zijn

…ossier into feat/SCHUL-611_notificatie-allegro-status-update

4c0n · 2026-01-26T13:10:27Z

src/Command/SyncDossierWithAllegroCommand.php

+        $io->info('Looking up allegro credentials');
+
+        /** @var OrganisatieRepository $organisatieRepository */
+        $organisatieRepository = $this->em->getRepository(Organisatie::class);


Het is beter om de repository te laten injecten ipv deze via de entity manager op te vragen.

4c0n · 2026-01-26T13:11:57Z

src/Command/SyncDossierWithAllegroCommand.php

+        /** @var OrganisatieRepository $organisatieRepository */
+        $organisatieRepository = $this->em->getRepository(Organisatie::class);
+
+        /** @var Organisatie $allegroId */


De type hint hier stelt dat het om een Organisatie gaat, maar de variabele naam geeft aan dat het om slechts een id gaat.

Ja inderdaad, ik had het overgenomen uit een ander Command. Ik pas het overal aan.

4c0n · 2026-01-26T13:12:40Z

src/Command/SyncDossierWithAllegroCommand.php

+            return Command::SUCCESS;
+        }
+
+        $dossierRepository = $this->em->getRepository(Dossier::class);


Het is beter om de repo to injecten in de constructor.

4c0n · 2026-01-26T13:13:09Z

src/Command/SyncDossierWithAllegroCommand.php

+
+        foreach ($dossiers as $dossier) {
+            $header = null;
+                try {


De indentatie loopt hier verkeerd.

src/Command/SyncDossierWithAllegroCommand.php

src/EventListener/MailNotitificationSubscriber.php

4c0n · 2026-01-26T13:23:23Z

src/EventListener/MailNotitificationSubscriber.php

+                'tokenStorage' => $this->tokenStorage
+            ]);
+        } else {
+            $this->logger->notice('Kan geen notifificatie sturen omdat er geen organisatie opgegeven is of er is voor de medewerker van dit dossier geen e-mailadres ingevuld', ['dossierId' => $dossier->getId(), 'gebruikerId' => $dossier->getMedewerkerOrganisatie() ? $dossier->getMedewerkerOrganisatie()->getId() : 'n/a']);


We weten op dit punt al dat er null komt uit getMedewerkerOriganisatie() toch? Waar dient deze conditie dan nog voor?

De code formatter heeft hier alleen de identatie aangepast. Ik raak deze functie verder niet aan.

4c0n · 2026-01-26T13:24:26Z

src/EventListener/MailNotitificationSubscriber.php

+                'dossier' => $dossier
+            ]);
+        } else {
+            $this->logger->notice('Kan geen notifificatie sturen omdat er geen organisatie opgegeven is of er is voor de medewerker van dit dossier geen e-mailadres ingevuld', ['dossierId' => $dossier->getId(), 'gebruikerId' => $dossier->getMedewerkerOrganisatie() ? $dossier->getMedewerkerOrganisatie()->getId() : 'n/a']);


We weten op dit punt al dat er null komt uit getMedewerkerOriganisatie() toch? Waar dient deze conditie dan nog voor?

Dit is een kopie van de bovenstaande functies. Ik zal kijken hoe ik het kan aanpassen.

4c0n · 2026-01-26T13:28:11Z

src/Service/AllegroService.php

     * @throws \Exception
     */
-    public function updateDossier(Dossier $dossier)
+    public function updateDossier(Dossier $dossier, TSRVAanvraagHeader $header = null)


Dit kan niet, je specificeert dat $header een object is van het type TSRVAanvraagHeader en daarna wijs je er null aan toe. Met die default value zou de type hint ?TSRVAanvraagHeader moeten zijn.

4c0n · 2026-01-26T13:29:03Z

src/Service/AllegroService.php

+     * @param Dossier $dossier
+     * @throws \Exception
+     */
+    public function isDossierInSyncWithAllegro(Dossier $dossier, TSRVAanvraagHeader $header = null)


Dit kan niet, je specificeert dat $header een object is van het type TSRVAanvraagHeader en daarna wijs je er null aan toe. Met die default value zou de type hint ?TSRVAanvraagHeader moeten zijn.

4c0n · 2026-01-26T13:29:51Z

src/Service/AllegroService.php

            ))
        )->getResult();
+
+        // TODO: event dispatchen waar de `MailNotificationSubscriber` naar luistert.


Aub de todo implementeren of de comment verwijderen.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

This PR adds an additional file action button for pdf files. This button opens a pdf inside the browser in a new tab. This button is only visible for PDF files HTML and CSS architecture is refactored. Instead of nested div with relative and absolut positions, the buttons are nested files action list container, which works with flexbox The CSS is made a bit more DRY by introducing mixin's and variables A new 'full-screen' icon is added

feat(db): drop legacy thumbnail table

…ossier into feat/SCHUL-611_notificatie-allegro-status-update

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

4c0n · 2026-01-27T14:43:13Z

src/EventListener/MailNotitificationSubscriber.php

+                'dossier' => $dossier
+            ]);
+        } else {
+            $this->logger->notice('Kan geen notifificatie sturen omdat er geen organisatie opgegeven is of er is voor de medewerker van dit dossier geen e-mailadres ingevuld', ['dossierId' => $dossier->getId(), 'gebruikerId' => $dossier->getMedewerkerOrganisatie() ? $dossier->getMedewerkerOrganisatie()->getId() : 'n/a']);


Suggested change

$this->logger->notice('Kan geen notifificatie sturen omdat er geen organisatie opgegeven is of er is voor de medewerker van dit dossier geen e-mailadres ingevuld', ['dossierId' => $dossier->getId(), 'gebruikerId' => $dossier->getMedewerkerOrganisatie() ? $dossier->getMedewerkerOrganisatie()->getId() : 'n/a']);

$this->logger->notice('Kan geen notifificatie sturen omdat er geen organisatie opgegeven is of er is voor de medewerker van dit dossier geen e-mailadres ingevuld', ['dossierId' => $dossier->getId(), 'gebruikerId' => 'n/a']);

--------- Co-authored-by: jmassink <j.massink@amsterdam.nl>

…ossier into feat/SCHUL-611_notificatie-allegro-status-update

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-30T11:51:43Z

templates/mails/notifyDossierSyncedWithAllegro.html.twig

+        <p>Hallo {{ dossier.getMedewerkerOrganisatie.getNaam }},</p>
+        <p>Een door jouw beheerd dossier heeft een statuswijziging gekregen vanuit de GKA.</p>
+        <p>&nbsp;</p>
+        <p><a href="{{ url('gemeenteamsterdam_fixxxschuldhulp_allegro_allegrosrveisers', {'dossierId': dossier.getId}) }}">Open dossier</a>


Corrected spelling of 'allegrosrveisers' to 'allegrosrvеisers' (assuming this is a typo in the route name).

Copilot · 2026-01-30T11:51:43Z

templates/mails/notifyDossierSyncedWithAllegro.html.twig

+        <p>Hallo {{ dossier.getMedewerkerOrganisatie.getNaam }},</p>
+        <p>Een door jouw beheerd dossier heeft een statuswijziging gekregen vanuit de GKA.</p>
+        <p>&nbsp;</p>
+        <p><a href="{{ url('gemeenteamsterdam_fixxxschuldhulp_allegro_allegrosrveisers', {'dossierId': dossier.getId}) }}">Open dossier</a>


Inconsistent method call syntax between HTML and text blocks. Line 12 uses property access without parentheses while line 21 uses method calls with parentheses. They should match for consistency and correctness.

Suggested change

Hallo {{ dossier.getMedewerkerOrganisatie.getNaam }},

Een door jouw beheerd dossier heeft een statuswijziging gekregen vanuit de GKA.

 

<a href="{{ url('gemeenteamsterdam_fixxxschuldhulp_allegro_allegrosrveisers', {'dossierId': dossier.getId}) }}">Open dossier</a>

Hallo {{ dossier.getMedewerkerOrganisatie().getNaam() }},

Een door jouw beheerd dossier heeft een statuswijziging gekregen vanuit de GKA.

 

<a href="{{ url('gemeenteamsterdam_fixxxschuldhulp_allegro_allegrosrveisers', {'dossierId': dossier.getId()}) }}">Open dossier</a>

Copilot · 2026-01-30T11:51:43Z

src/Service/AllegroService.php

+        $incomingStatus = $header?->getStatus();
+        $incomingExtraStatus = $header?->getExtraStatus();
+
+        return $currentStatus !== $incomingStatus || $currentExtraStatus !== $incomingExtraStatus;


The method name isDossierInSyncWithAllegro is misleading as it returns true when the dossier is NOT in sync (statuses differ). Consider renaming to isDossierOutOfSyncWithAllegro or hasDossierStatusChanged.

Suggested change

return $currentStatus !== $incomingStatus || $currentExtraStatus !== $incomingExtraStatus;

return $currentStatus === $incomingStatus && $currentExtraStatus === $incomingExtraStatus;

Copilot · 2026-01-30T11:51:44Z

src/Command/SyncDossierWithAllegroCommand.php

+        $allegroUser = $this->allegroCommandHelper->getAllegroUserFromAnyOrg();
+
+        if (!isset($allegroUser)) {
+            $io->info('No organistation found whith a complete set of Allegro login data');


Corrected spelling of 'organistation' to 'organisation' and 'whith' to 'with'.

Suggested change

$io->info('No organistation found whith a complete set of Allegro login data');

$io->info('No organisation found with a complete set of Allegro login data');

Copilot · 2026-01-30T11:51:44Z

src/Command/ImportSchuldeisersAllegroCommand.php


-        if (null === $allegroId) {
+        if (!isset($allegroUser)) {
            $io->info('No organistation found whith a complete set of Allegro login data');


Corrected spelling of 'organistation' to 'organisation' and 'whith' to 'with'.

Suggested change

$io->info('No organistation found whith a complete set of Allegro login data');

$io->info('No organisation found with a complete set of Allegro login data');

Copilot · 2026-01-30T11:51:44Z

src/Command/AllegroCommandHelper.php

+    public function __construct(
+        private EntityManagerInterface $em,
+    ) {
+        $this->em = $em;


The assignment $this->em = $em on line 14 is redundant since the parameter is already promoted to a property using private EntityManagerInterface $em in the constructor parameter list.

Suggested change

$this->em = $em;

Copilot · 2026-01-30T11:51:44Z

src/Command/SyncDossierWithAllegroCommand.php

+        $this->service = $service;
+        $this->allegroCommandHelper = $allegroCommandHelper;
+        $this->eventDispatcher = $eventDispatcher;


Lines 36-38 contain redundant assignments since the parameters are already promoted to properties using constructor property promotion. Only line 35 is needed.

Suggested change

$this->service = $service;

$this->allegroCommandHelper = $allegroCommandHelper;

$this->eventDispatcher = $eventDispatcher;

Copilot · 2026-01-30T11:51:45Z

src/EventListener/MailNotitificationSubscriber.php

+        }
+    }
+
+    public function notifyDossierSyncedWithAllegro(DossierChangedEvent $event)


The parameter type should be DossierSyncedWithAllegroEvent instead of DossierChangedEvent to properly match the event being dispatched and maintain type safety.

Suggested change

public function notifyDossierSyncedWithAllegro(DossierChangedEvent $event)

public function notifyDossierSyncedWithAllegro(DossierSyncedWithAllegroEvent $event)

RubenSibon and others added 12 commits January 9, 2026 14:17

build(release): file w/ tag

9862362

build(Make): re-add node versions

527dae9

build(xdebug): client host

6608efd

Merge branch 'release/SCHUL_sprint-7' of github.com:Amsterdam/schuldd…

18e82f5

…ossier into release/SCHUL_sprint-7

chore: trivial space addition

ae627aa

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Merge branch 'release/SCHUL_sprint-7' of github.com:Amsterdam/schuldd…

c76fa15

…ossier into release/SCHUL_sprint-7

build(config): add Editor Config

b0250b4

Feat(schul-735): update .editorconfig (#379)

715f039

style(editorconfig): update linelength

feat: check, sync and notify for GKA refresh

ffe998c

Merge branch 'release/SCHUL_sprint-7' of github.com:Amsterdam/schuldd…

2c17d0a

…ossier into feat/SCHUL-611_notificatie-allegro-status-update

Copilot AI review requested due to automatic review settings January 22, 2026 13:47

jmassink reviewed Jan 22, 2026

View reviewed changes

src/Command/SyncDossierWithAllegroCommand.php Outdated Show resolved Hide resolved

jmassink reviewed Jan 22, 2026

View reviewed changes

src/Command/SyncDossierWithAllegroCommand.php Show resolved Hide resolved

jmassink reviewed Jan 22, 2026

View reviewed changes

src/Command/SyncDossierWithAllegroCommand.php Show resolved Hide resolved

Copilot AI reviewed Jan 22, 2026

View reviewed changes

RubenSibon added 2 commits January 23, 2026 12:03

feat(mail): template and attempt to dispatch event

723c245

refactor: get header and call sync and update

cc816d3

RubenSibon requested review from 4c0n, Copilot and tvanoort85 January 23, 2026 14:21

RubenSibon self-assigned this Jan 23, 2026

RubenSibon marked this pull request as ready for review January 23, 2026 14:23

feat(SCHUL-662): only admin can see export to allegro button (#381)

29d5607

feat(dossierDasboard.html.twig): don't show export button to normal users

Copilot AI reviewed Jan 23, 2026

View reviewed changes

tvanoort85 previously approved these changes Jan 23, 2026

View reviewed changes

RubenSibon added 3 commits January 23, 2026 16:50

refactor: and new query to repo

bd58207

refactor: more exact error handling

350ff1d

Merge branch 'release/SCHUL_sprint-7' of github.com:Amsterdam/schuldd…

eacbeb3

…ossier into feat/SCHUL-611_notificatie-allegro-status-update

4c0n reviewed Jan 26, 2026

View reviewed changes

src/Command/SyncDossierWithAllegroCommand.php Outdated Show resolved Hide resolved

4c0n reviewed Jan 26, 2026

View reviewed changes

src/EventListener/MailNotitificationSubscriber.php Show resolved Hide resolved

4c0n reviewed Jan 26, 2026

View reviewed changes

refactor: allegro command helper

637e7ee

Copilot AI review requested due to automatic review settings January 26, 2026 13:27

4c0n reviewed Jan 26, 2026

View reviewed changes

Copilot AI reviewed Jan 26, 2026

View reviewed changes

jmassink and others added 6 commits January 27, 2026 10:13

Feat(SCHUL-736): drop thumbnail table (#382)

cd9f44a

feat(db): drop legacy thumbnail table

fix(type): change var name to reflect type

5801234

refactor: inject repo in constructor

e74eed7

Merge branch 'release/SCHUL_sprint-7' of github.com:Amsterdam/schuldd…

3806055

…ossier into feat/SCHUL-611_notificatie-allegro-status-update

fix: make param optional

bba49c1

Copilot AI review requested due to automatic review settings January 27, 2026 12:46

Copilot AI reviewed Jan 27, 2026

View reviewed changes

4c0n reviewed Jan 27, 2026

View reviewed changes

RubenSibon and others added 4 commits January 27, 2026 15:55

Release notes & release banner (#384)

cdc43fe

--------- Co-authored-by: jmassink <j.massink@amsterdam.nl>

Merge branch 'release/SCHUL_sprint-7' of github.com:Amsterdam/schuldd…

a639993

…ossier into feat/SCHUL-611_notificatie-allegro-status-update

refactor: rename fn to reflect type

73c0104

chore: address comments

b417edb

Copilot AI review requested due to automatic review settings January 30, 2026 11:50

Copilot AI reviewed Jan 30, 2026

View reviewed changes

Base automatically changed from release/SCHUL_sprint-7 to main February 3, 2026 08:44

	$this->logger->notice('Kan geen notifificatie sturen omdat er geen organisatie opgegeven is of er is voor de medewerker van dit dossier geen e-mailadres ingevuld', ['dossierId' => $dossier->getId(), 'gebruikerId' => $dossier->getMedewerkerOrganisatie() ? $dossier->getMedewerkerOrganisatie()->getId() : 'n/a']);
	$this->logger->notice('Kan geen notifificatie sturen omdat er geen organisatie opgegeven is of er is voor de medewerker van dit dossier geen e-mailadres ingevuld', ['dossierId' => $dossier->getId(), 'gebruikerId' => 'n/a']);

	return $currentStatus !== $incomingStatus \|\| $currentExtraStatus !== $incomingExtraStatus;
	return $currentStatus === $incomingStatus && $currentExtraStatus === $incomingExtraStatus;

	$io->info('No organistation found whith a complete set of Allegro login data');
	$io->info('No organisation found with a complete set of Allegro login data');

	$this->service = $service;
	$this->allegroCommandHelper = $allegroCommandHelper;
	$this->eventDispatcher = $eventDispatcher;

	public function notifyDossierSyncedWithAllegro(DossierChangedEvent $event)
	public function notifyDossierSyncedWithAllegro(DossierSyncedWithAllegroEvent $event)

Conversation

RubenSibon commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

tvanoort85 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

RubenSibon commented Jan 22, 2026 •

edited

Loading