Skip to content
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

LCH-6790-2.x : Port maintenance mode changes #187

Closed
wants to merge 24 commits into from
Closed

Conversation

kirtigarg2584
Copy link

Motivation

Fixes #NNN

Proposed changes

Alternatives considered

Testing steps

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need the logging client in 2.x version of the module, because it doesn't utilize the event service. That only become available in 3.x version of the module.

Copy link
Contributor

Choose a reason for hiding this comment

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

And we are not going to need this either.

*
* @var \Psr\Http\Message\ResponseInterface
*/
private ResponseInterface $response;
Copy link
Contributor

Choose a reason for hiding this comment

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

In this version of the library we still support 7.3. That would break backward compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need that class either, because it is only used in ContentHubLoggingClient.

Copy link
Contributor

Choose a reason for hiding this comment

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

We dont' need this either, because it is related to event service.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this too.

*
* @var \Psr\Http\Message\ResponseInterface
*/
private ResponseInterface $response;
Copy link
Contributor

Choose a reason for hiding this comment

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

In this version of the library we still support 7.3. That would break backward compatibility.

Comment on lines +1269 to +1348
/**
* Returns an extended interest list based on the site role.
*
* @param string $webhook_uuid
* Identifier of the webhook.
* @param string $site_role
* The role of the site.
* @param bool|null $disable_syndication
* Filter for disabled entities.
* If set to true, only disabled entities will be returned.
* If set to false, only enabled entities will be returned.
* If not set, all the entities will be returned.
*
* @return array
* An associate array keyed by the entity uuid.
*
* @throws \Exception
*/
public function getInterestsByWebhookAndSiteRole(string $webhook_uuid, string $site_role, ?bool $disable_syndication = NULL): array {
$options = [];
if (isset($disable_syndication)) {
$options['query'] = [
'disable_syndication' => $disable_syndication,
];
}
$data = self::getResponseJson($this->get("interest/webhook/$webhook_uuid/$site_role", $options));
return $data['data'] ?? [];
}

/**
* The extended interest list to add based on site role.
*
* Format:
* [
* 'fe5f27d1-6e41-4609-b65a-2cb179549d1e' => [
* 'status' => '',
* 'reason' => '',
* 'event_ref' => '',
* ],
* ]
*
* @param string $webhook_uuid
* The webhook uuid to register interest items for.
* @param string $site_role
* The site role.
* @param array $interest_list
* An array of interest items.
*
* @return \Psr\Http\Message\ResponseInterface
* Response object.
*/
public function addEntitiesToInterestListBySiteRole(string $webhook_uuid, string $site_role, array $interest_list): ResponseInterface {
$options['body'] = json_encode($interest_list);

return $this->post("interest/webhook/$webhook_uuid/$site_role", $options);
}

/**
* The extended interest list to add based on site role.
*
* Format:
*
* @see \Acquia\ContentHubClient\ContentHubClient::addEntitiesToInterestListBySiteRole
*
* @param string $webhook_uuid
* The webhook uuid to register interest items for.
* @param string $site_role
* The site role.
* @param array $interest_list
* An array of interest items.
*
* @return \Psr\Http\Message\ResponseInterface
* Response object.
*/
public function updateInterestListBySiteRole(string $webhook_uuid, string $site_role, array $interest_list): ResponseInterface {
$options['body'] = json_encode($interest_list);

return $this->put("interest/webhook/$webhook_uuid/$site_role", $options);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

These can be removed, we are not using these functions. They're supposed to only exist in 3.x version.

Copy link
Contributor

@narendradesai narendradesai left a comment

Choose a reason for hiding this comment

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

There are unnecessary file changes in this PR. Need to refactor it. Will work on this in morning.

*
* @var \Psr\Log\LoggerInterface
*/
public LoggerInterface $logger;
Copy link
Contributor

Choose a reason for hiding this comment

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

In this version of the library we still support 7.3. That would break backward compatibility.

Copy link
Contributor

@saxumVermes saxumVermes left a comment

Choose a reason for hiding this comment

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

@kirtigarg2584 By looking through the code, tbh I think the only necessary change in this PR is to add the new request observer function through which we can retrieve the last response.

@narendradesai
Copy link
Contributor

@saxumVermes @kirtigarg2584 , I've created a separate PR for Porting maintenance mode changes to 2.x: #191

@saxumVermes saxumVermes closed this Dec 5, 2023
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.

3 participants