Skip to content

feat: cloud API - #102

Open
supun-io wants to merge 21 commits into
mainfrom
feat/cloud-api
Open

feat: cloud API#102
supun-io wants to merge 21 commits into
mainfrom
feat/cloud-api

Conversation

@supun-io

@supun-io supun-io commented Jul 18, 2026

Copy link
Copy Markdown
Member

@supun-io

Copy link
Copy Markdown
Member Author

@Nadil-K review needed:

  • a critical review of src/CloudApi/ConsoleApiAuth/ConsoleApiAuthorizationListenerAbstract.php and it's tests (tests span across 4 files)
  • a thorough review of all CloudApi/ classes and tests
  • a general review of all the changes in the PR


public function from(): array
{
return [];

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • why [] ?

Comment thread tests/SymfonyTestCase.php
$this->em->clear();
}

public function createMock(string $originalClassName): MockObject

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • What is the reason to replace this with createStub()?

use Hyvor\Internal\Tests\Unit\CloudApi\CloudApiAuth\ConsoleApiAuthTrait;
use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • Add #[CoversClass()]

use PHPUnit\Framework\Attributes\CoversClass;

#[CoversClass(ScopeBuilder::class)]
class ScopeBuilderTest extends SymfonyTestCase

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • Cover the following paths:
  • ->addScopes(Component::TALK, [TalkScope::WEBSITE_READ, TalkScope::WEBSITE_WRITE, PostScope::NEWSLETTER_READ])
  • ->fromScopeString('talk:website.read invalid-component:website.write post:newsletter.read')

if (!Component::tryFrom($component)) {
continue;
}
$scopeBuilder->scopes[$component][] = $scopeName;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • Shouldn't we validate the scope against the component here?


public function __invoke(ControllerEvent $event): void
{
if (!str_starts_with($event->getRequest()->getPathInfo(), $this->getBasePath())) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

$request = $event->getRequest();
$path = $request->getPathInfo();
  • Use these variables below. No need to call the same function multiple times.


private function handleResourceApiKey(ControllerEvent $event, string $apiKey): ConsoleAuthResults
{
$orgEndpoint = $event->getAttributes(OrgEndpoint::class)[0] ?? null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

count($event->getAttributes(OrgEndpoint::class)) > 0


private function handleCloudToken(ControllerEvent $event, string $jwtToken): ConsoleAuthResults
{
$orgEndpoint = $event->getAttributes(OrgEndpoint::class)[0] ?? null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

count($event->getAttributes(OrgEndpoint::class)) > 0

private function handleSession(ControllerEvent $event): ConsoleAuthResults
{
$request = $event->getRequest();
$orgEndpoint = $event->getAttributes(OrgEndpoint::class)[0] ?? null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

count($event->getAttributes(OrgEndpoint::class)) > 0

*/
public function resolve(Request $request, ArgumentMetadata $argument): iterable
{
// get the argument type (e.g. BookingId)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • BookingId ??

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.

2 participants