rudder-plugins icon indicating copy to clipboard operation
rudder-plugins copied to clipboard

Fixes #26906: Migrate the ChangeValidationSettings snippet from Scala/lift to Elm

Open skaerg opened this issue 7 months ago • 1 comments

https://issues.rudder.io/issues/26906

skaerg avatar May 15 '25 15:05 skaerg

PR updated with a new commit

skaerg avatar May 16 '25 10:05 skaerg

Commit modified

skaerg avatar May 22 '25 14:05 skaerg

Commit modified

skaerg avatar Jun 04 '25 09:06 skaerg

Two of the info sections contradict each other.

  • The "Configure change request triggers" section reads : "Be careful: a change request is created when at least one predicate matches, so an exempted user still need a change request in order to edit a node from a supervised group. ",
  • While the "Configure users with change validation" reads : "Hence, configuring the [supervised] groups below will have no effect on validated users (in the list above), but will apply to non-validated users, who will still need to create a change request in order to modify a node from a supervised group. "

image image

In addition, the "Supervised targets"' info section has an unfinished phrase : "Change validation are enable for any change that would impact a node belonging to one of the chosen groups below. Be careful: a change on one another group "

image

Should the info sections be modified ?

skaerg avatar Jun 06 '25 07:06 skaerg

yes you can do improvements on the HTML content, it was in the HTML template but now it's in an Elm file so if you don't do it in Elm, it will not be fixed (if we also fix it in previous version with HTML, it will need to be fixed by hand in the Elm file during upmerge)

clarktsiory avatar Jun 06 '25 08:06 clarktsiory

I don't know which one is correct, so it needs to be tested

clarktsiory avatar Jun 06 '25 08:06 clarktsiory

The only other tasks left that I can think of for this PR are :

  • Maybe changing some of the contents of the info-sections : see https://github.com/Normation/rudder-plugins/pull/832#issuecomment-2948416828
  • The checkboxes in the WorkflowUsers app are not styled : they are default checkboxes unlike the rest of the page, which uses the fa fa-check class. Should these checkboxes also use fa fa-check ?

skaerg avatar Jun 10 '25 07:06 skaerg

  • For the info section, this is not urgent, as it impacts previous versions first, so later we would make a dedicated fix for that
  • For checkboxes, ideally we should normalize using bootstrap 5 checkboxes and we would need to synchronize the Elm model with the inputs, but if it's too complicated we can keep it as it is

clarktsiory avatar Jun 10 '25 08:06 clarktsiory

OK, merging this PR