chore: Improvements at response from API when generate root certificate by Any97Cris · Pull Request #5198 · LibreSign/libresign

Any97Cris · 2025-07-04T18:27:58Z

Pull Request Description

Related Issue

Issue Number: #1967

Pull Request Type

Chore

vitormattos

Now you need to recreate src/helpers/certification.js to send it to the front end. I think the best way is via an initial state, perhaps named certification_rules.

It might be a good idea to create a toArray method in the RulesServices class. This could be a good way to transform the PHP code into JavaScript Object Notation and send it serialized to the front end via the initial state.

And then, remove the src/helpers/certification.js matching the suggestion that was exposed at the issue:

To do this you will need to move the file apps-extra/libresign/src/helpers/certification.js to be a helper in API side and return this as JSON in an initial state to frontend.

lib/Service/Certificate/RulesService.php

tests/php/Unit/Service/Certificate/RulesServiceTest.php

AntDavidLima

Everything looks right to me. Great job!
I've just requested some changes related to some minor, not that important things.

AntDavidLima · 2025-08-29T13:28:37Z

lib/Controller/AdminController.php

-					throw new LibresignException('Parameter id is required!', 400);
-				}
-				$names[$item['id']]['value'] = $this->trimAndThrowIfEmpty($item['id'], $item['value']);
+				$names[$item['id']]['value'] = trim((string)$item['value']);


It may be good to rewrite this line in multiple lines, to make it easier to read. Something like:

$itemId = $item['id']; $itemValue = strval($item['value']); $names[$itemId]['value'] = trim($itemValue);

It's something silly, but may help people new to the application to understand it's code.

Isn't this too much vars?

Maybe would be best create a fallback issue to implement a NormalizeService class to add the trim rule into all values and also add an uppercase rule into Country and maintain the scope of this PR more amalgamated with the issue that motivated this PR.

AntDavidLima · 2025-08-29T13:35:13Z

lib/Service/Certificate/ValidateService.php

+		if ($length > $rule['max'] || $length < $rule['min']) {
+			throw new InvalidArgumentException(
+				$this->l10n->t(
+					"Parameter '%s' should be between %s and %s.",


Parameter '%s' should be between %s and %s characters. will be clearer, I think.

characters only make sense when is about a string. The interval is numeric.

Signed-off-by: Crisciany Souza <crisciany.souza@librecode.coop>

Signed-off-by: Crisciany Souza <crisciany.souza@librecode.coop> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

I changed because the previous scenaio name was a copy paste of first scenario. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

The entire name of this field is Common Name and not only Name. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

vitormattos · 2025-09-01T12:41:21Z

I only made a rebase without any change at commits of this PR

vitormattos · 2025-09-01T13:05:03Z

lib/Controller/AdminController.php

-					throw new LibresignException('Parameter id is required!', 400);
-				}
-				$names[$item['id']]['value'] = $this->trimAndThrowIfEmpty($item['id'], $item['value']);
+				$names[$item['id']]['value'] = trim((string)$item['value']);


Maybe would be best create a fallback issue to implement a NormalizeService class to add the trim rule into all values and also add an uppercase rule into Country and maintain the scope of this PR more amalgamated with the issue that motivated this PR.

vitormattos · 2025-09-01T14:49:19Z

/backport to stable31

vitormattos · 2025-09-01T14:49:21Z

/backport to stable30

Any97Cris self-assigned this Jul 4, 2025

github-project-automation bot moved this to 0. Needs triage in Roadmap Jul 4, 2025

github-project-automation bot added this to Roadmap Jul 4, 2025

vitormattos changed the title ~~chore: scenery validate test~~ chore: scenary validate test Jul 7, 2025

vitormattos moved this from 0. Needs triage to 2. in progress in Roadmap Jul 9, 2025

vitormattos self-requested a review July 9, 2025 01:03

This comment was marked as resolved.

Sign in to view

github-project-automation bot moved this from 2. in progress to 1. to do in Roadmap Jul 9, 2025

vitormattos moved this from 1. to do to 2. in progress in Roadmap Jul 9, 2025

vitormattos force-pushed the chore/correct-validate-field branch from 195b468 to abfb7e6 Compare July 22, 2025 20:17

This comment was marked as resolved.

Sign in to view

vitormattos reviewed Aug 18, 2025

View reviewed changes

Any97Cris force-pushed the chore/correct-validate-field branch 2 times, most recently from 77b64e6 to 82548ab Compare August 21, 2025 20:11

vitormattos reviewed Aug 21, 2025

View reviewed changes

lib/Service/Certificate/RulesService.php Outdated Show resolved Hide resolved

lib/Service/Certificate/RulesService.php Outdated Show resolved Hide resolved

vitormattos reviewed Aug 21, 2025

View reviewed changes

tests/php/Unit/Service/Certificate/RulesServiceTest.php Show resolved Hide resolved

tests/php/Unit/Service/Certificate/RulesServiceTest.php Show resolved Hide resolved

Any97Cris force-pushed the chore/correct-validate-field branch 3 times, most recently from cb4a7ad to 148d991 Compare August 28, 2025 20:23

Any97Cris requested review from AntDavidLima and vitormattos August 29, 2025 13:11

AntDavidLima requested changes Aug 29, 2025

View reviewed changes

github-project-automation bot moved this from 2. in progress to 1. to do in Roadmap Aug 29, 2025

Any97Cris and others added 7 commits September 1, 2025 09:35

chore: scenery validate test

4754556

Signed-off-by: Crisciany Souza <crisciany.souza@librecode.coop>

chore: more steps about tests

4e81367

Signed-off-by: Crisciany Souza <crisciany.souza@librecode.coop> Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

fix: name of scenario

8295004

I changed because the previous scenaio name was a copy paste of first scenario. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

fix: adjust table row size

2d84d6a

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

chore: replace Name by Common Name

b363c58

The entire name of this field is Common Name and not only Name. Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

chore: restrict the return type to nullable and string only

07de6ef

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

chore: cover with more tests

fb27977

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

vitormattos added 8 commits September 1, 2025 09:35

chore: test get helper text with invalid field name

6f6fc9b

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

chore: test get rule to valid field

a276363

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

chore: test get rule to invalid field

4b2d15d

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

chore: add type hint to return

1527625

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

fix: consider to return empty array when is invalid field name

3d5c742

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

fix: run cs:fix

9e2e88f

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

fix: run cs:fix

4da6a72

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

chore: simplify test and compare actual and expected directly

4b43967

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>

vitormattos force-pushed the chore/correct-validate-field branch from 5740a33 to 4b43967 Compare September 1, 2025 12:40

vitormattos approved these changes Sep 1, 2025

View reviewed changes

vitormattos changed the title ~~chore: scenary validate test~~ chore: Improvements at response from API when generate root certificate Sep 1, 2025

vitormattos merged commit 29392a3 into main Sep 1, 2025
59 checks passed

vitormattos deleted the chore/correct-validate-field branch September 1, 2025 14:49

github-project-automation bot moved this from 1. to do to 4. to release in Roadmap Sep 1, 2025

backportbot-libresign bot added the backport-request label Sep 1, 2025

backportbot-libresign bot mentioned this pull request Sep 1, 2025

[stable31] chore: Improvements at response from API when generate root certificate #5375

Merged

backportbot-libresign bot removed the backport-request label Sep 1, 2025

backportbot-libresign bot mentioned this pull request Sep 1, 2025

[stable30] chore: Improvements at response from API when generate root certificate #5376

Merged

Uh oh!

Conversation

Any97Cris commented Jul 4, 2025

Pull Request Description

Related Issue

Pull Request Type

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

vitormattos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AntDavidLima left a comment

Choose a reason for hiding this comment

Uh oh!

AntDavidLima Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

vitormattos Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

vitormattos Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

AntDavidLima Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

vitormattos Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vitormattos commented Sep 1, 2025

Uh oh!

vitormattos Sep 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vitormattos commented Sep 1, 2025

Uh oh!

vitormattos commented Sep 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vitormattos Sep 1, 2025 •

edited

Loading