Skip to content

Replace legacy entrypoints with controllers - #380

Merged
btry merged 8 commits into
pluginsGLPI:12.0/bugfixesfrom
btry:feature/use_controllers
Oct 15, 2026
Merged

btry merged 8 commits into
pluginsGLPI:12.0/bugfixesfrom
btry:feature/use_controllers

Conversation

@btry

@btry btry commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@btry btry added the enhancement New feature or request label Oct 9, 2026
@btry
btry force-pushed the feature/use_controllers branch from 299beb2 to 9bcd9c2 Compare October 9, 2026 11:08
@btry
btry requested a review from Rom1-B October 9, 2026 11:18

@Rom1-B Rom1-B left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add functional tests for both controllers, at least the rights checks on reset and calculate?

case $request->request->has('calculate'):
return $this->calculate($request);
default:
throw new Exception('Method not allowed');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unknown action returns a 500; a 400 fits better.

Suggested change
throw new Exception('Method not allowed');
throw new BadRequestHttpException();

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

case $request->request->has('calculate'):
return $this->calculate($request);
default:
throw new Exception('Method not allowed', 405);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as in EmbodiedImpactController.

Suggested change
throw new Exception('Method not allowed', 405);
throw new BadRequestHttpException();

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

Comment on lines +62 to +65
// if method is GET, then throw an exception, workaround bug in GLPI up to 11.0.7
if ($request->isMethod('GET')) {
return new Response('', 403);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this workaround still needed with GLPI >= 12.0.0? If not, methods: ['POST'] would be enough; otherwise the comment says "throw" but the code returns a 403.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is no longer needed. Removed

Comment thread src/Controller/EmbodiedImpactController.php
@btry

btry commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Out of scope, but added tests for other controllers recently created

@btry
btry requested a review from Rom1-B October 13, 2026 08:20
@btry
btry force-pushed the feature/use_controllers branch from 40308da to d4bf980 Compare October 13, 2026 08:33
@btry
btry force-pushed the feature/use_controllers branch 2 times, most recently from d4bf980 to 82b9804 Compare October 13, 2026 08:35
@btry
btry force-pushed the feature/use_controllers branch 2 times, most recently from caab88b to 87a2bb8 Compare October 13, 2026 12:54
@btry
btry force-pushed the feature/use_controllers branch from 87a2bb8 to 9988363 Compare October 13, 2026 12:56
@btry
btry force-pushed the feature/use_controllers branch from 6a0c09e to 9d359a7 Compare October 13, 2026 13:14
Comment thread src/Controller/EmbodiedImpactController.php
Comment thread src/Controller/EmbodiedImpactController.php
Comment thread src/Controller/UsageImpactController.php
@btry
btry requested a review from Rom1-B October 15, 2026 07:46
@btry
btry merged commit f507ba7 into pluginsGLPI:12.0/bugfixes Oct 15, 2026
8 checks passed
@btry
btry deleted the feature/use_controllers branch October 15, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants