Skip to content

Conversation

rjzondervan
Copy link
Contributor

No description provided.

rjzondervan and others added 29 commits September 24, 2024 16:42
warning: this will cause infinite loops. Working on that
Copy link

github-actions bot commented Oct 4, 2024

👋 @rjzondervan
Thank you for raising your pull request.
Please make sure you have followed our contributing guidelines. We will review it as soon as possible. In the meanwhile make sure your PR checks the following boxes

  • Is based on an issue
  • Has been locally tested
  • Has been tested with the admin UI
  • Has been discussed with the development team in an open channel

Copy link

@WilcoLouwerse WilcoLouwerse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -0,0 +1,34 @@
{
"title": "Sync Cases from VrijBRP",
"$id": "https://commongateway.nl/action/vrijbrp.synczaken.action.json",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use vrijbrp.nl here instead

different domain than mappings from https://github.com/CommonGateway/VrijBRPToZGWBundle/pull/1/files
might be better to be consistent with these references

https://github.com/CommonGateway/VrijBRPToZGWBundle/pull/2/files#r1787422666

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the domain from https://github.com/CommonGateway/VrijBRPToZGWBundle/pull/1/files, that's exactly why I changed the domains to commongateway.nl

@@ -0,0 +1,21 @@
{
"title": "CreateCaseNotification",
"$id": "https://commongateway.nl/action/vrijbrp.createCaseNotification.action.json",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use vrijbrp.nl here instead

@@ -0,0 +1,21 @@
{
"title": "CreateStatusNotification",
"$id": "https://commongateway.nl/action/vrijbrp.createStatusNotification.action.json",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use vrijbrp.nl here instead

*
* @return Synchronization The updated synchronization
*/
public function synchronizeFromSource(Synchronization $synchronization, array $sourceObject=[], bool $unsafe=false): Synchronization

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kan naar deze functie gekeken worden? Aardig wat warnings erop? Complexiteit etc

$this->synchronizationLogger->info("handleSync for Synchronization with id = {$synchronization->getId()->toString()}");

// create new object if no object exists
if (!$synchronization->getObject()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kan !== null


if ($resultDot->has(keys: $configuration['resultsPath']) === true) {
$return = $resultDot->get(key: $configuration['resultsPath']);
if ($return instanceof Dot) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kan === true

$return = $resultDot->get(key: $configuration['resultsPath']);
if ($return instanceof Dot) {
return $return->jsonSerialize();
} else if (is_array($return)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kan === true

{
$object = $data['object'];

if ($object instanceof ObjectEntity) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kan === true

@rjzondervan rjzondervan merged commit 96c1d9c into main Oct 4, 2024
8 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants