rudder icon indicating copy to clipboard operation
rudder copied to clipboard

Fixes #24634: API to find usage of a node property in Directives

Open ElaadF opened this issue 1 year ago • 25 comments

https://issues.rudder.io/issues/24634

https://github.com/Normation/rudder/assets/23410978/af26fc64-3942-432a-a6c1-026d8d8f38ba

https://github.com/Normation/rudder/assets/23410978/3b9cc7a5-0547-4aff-9a02-d955176042db

Preamble

This PR has been made as a side project on my free time. I don't have a deadline, and I don't know when it will be done. It's started as a small test to see if this was not too much painful to develop, but it ends up being a little bit more polish that I was expecting. Feel free to comment, add remarks and review the work done, I will do my best to address them ASAP on my free time.

Goal

This feature aim to add discoverability on node's properties. Nodes' properties are an essential part of Rudder workflow, it's seem important to see if a property used or not and if yes to be able to visit the object that is using this property. I'm often

Journal

24/06/2025 - https://github.com/Normation/rudder/pull/5557/commits/dc02d3d4eb548cc464e64a421806280754d868b3 https://github.com/Normation/rudder/pull/5557/commits/4a35c531f9d85d9ff23477c6b48aff6447a80f94

  • Modify the Elm App to use the quicksearch API to find property usage. Initially, I have created a dedicated API that get all directives and techniques and was searching inside their parameters.
  • Removing the searching logic in Scala that is no more used as the unit testing part

29/05/2024 - https://github.com/Normation/rudder/pull/5557/commits/1991710a3f09f1f5408e973d9cbdb2dd2c8ec124

  • The modal was not popping when clicking to the icon due to retargetting in 8.2, it was using the modal style (8.0 and before)
  • Change the presentation video to reflect all the changes

15/05/2024 - https://github.com/Normation/rudder/pull/5557/commits/92ab292047d2eed981f676f42685a426655f98a4 https://github.com/Normation/rudder/pull/5557/commits/96765a7c982dbe5691f067f353922119f2e648f4

  • Retargeting in 8.2 (master since the branch doesn't exist for the moment)
  • Add policyMode missing parameter from test objects MethodBlocks EditorTechnique
  • Fix some indentation issues introduced by the retargeting to 8.2

16/04/2024 - https://github.com/Normation/rudder/pull/5557/commits/92ab292047d2eed981f676f42685a426655f98a4 https://github.com/Normation/rudder/pull/5557/commits/3e851459f79a75184fe6dd8ca7292945af3b1bab

  • Beautify the search icon, to make it consistent with the edit buttons, only appear when the table line is hovered
  • Make the find usage API private
  • Next step verify that a user with no required read rights doesn't have access to this feature

06/04/2024 - https://github.com/Normation/rudder/pull/5557/commits/89348fd8e3c55695368cd519b24de5174740ad1e

  • Pagination is added in the UI (see the gif) it was a bit tedious, the hardest part was to make the table scrollable and contains inside the modal, I have some ugly CSS to force the max-height and the rows width but... it works, that's all I need
  • Next step is to beautify the search button to display it only on hover

05/04/2024

  • I have been able to set up API and service testing, the logics are a bit redundant, but the API is here to make sure that it is responding as expecting, prevent breaking the UI. In the service testing, we can see what the service take into account for detecting a property's usage.
  • Next step is probably adding pagination to the UI, because some user can have a lot of Technique and Directive, we have to be able to scale in terms of UI for the table since this is a tiny popup. I'm also concerned about the performance issues to parse every directive's parameter and technique's parameter on large instance, need more reflection on this subject, later.

04/04/2024 - https://github.com/Normation/rudder/pull/5557/commits/fe6096f934c3b9ee971c83efe4cdcae728c676b8

  • The feature is working as expected, I have been able to add API tests, but I have some issue to make working my service tests, apparently test_find_property_usage_in_technique is not initialized, making some other tests (who rely on NodeConfig) fail, and I cannot figure it why for the moment.

Todos :

  • [x] Add pagination https://github.com/Normation/rudder/pull/5557/commits/89348fd8e3c55695368cd519b24de5174740ad1e
  • [x] Unit testing https://github.com/Normation/rudder/pull/5557/commits/83ae097c824caf0d3d6e2e5140d09e00fbafe52c
  • [x] Beautify find usage button https://github.com/Normation/rudder/pull/5557/commits/3e851459f79a75184fe6dd8ca7292945af3b1bab
  • [x] Make the API private https://github.com/Normation/rudder/pull/5557/commits/92ab292047d2eed981f676f42685a426655f98a4 -> Need confirmation from @VinceMacBuche
  • [x] Rebase in 8.2 (master)
  • [ ] discuss about how to detect usage of a property see https://github.com/Normation/rudder/pull/5557/files#r1613155779
  • [ ] Verify Read access requirements for URL redirection and find usage
  • [ ] Removing fake data for pagination testing in Elm
  • [ ] Load testing
  • [ ] Refactoring toJson

ElaadF avatar Mar 29 '24 22:03 ElaadF

PR updated with a new commit

ElaadF avatar Mar 29 '24 22:03 ElaadF

PR updated with a new commit

ElaadF avatar Mar 29 '24 22:03 ElaadF

PR updated with a new commit

ElaadF avatar Mar 31 '24 21:03 ElaadF

PR updated with a new commit

ElaadF avatar Mar 31 '24 21:03 ElaadF

Commit modified

ElaadF avatar Mar 31 '24 22:03 ElaadF

PR rebased

ElaadF avatar Mar 31 '24 22:03 ElaadF

PR rebased

ElaadF avatar Mar 31 '24 22:03 ElaadF

PR updated with a new commit

ElaadF avatar Apr 01 '24 01:04 ElaadF

Commit modified

ElaadF avatar Apr 01 '24 01:04 ElaadF

Commit modified

ElaadF avatar Apr 01 '24 01:04 ElaadF

PR updated with a new commit

ElaadF avatar Apr 01 '24 13:04 ElaadF

Commit modified

ElaadF avatar Apr 01 '24 13:04 ElaadF

Commit modified

ElaadF avatar Apr 01 '24 14:04 ElaadF

Commit modified

ElaadF avatar Apr 01 '24 14:04 ElaadF

Commit modified

ElaadF avatar Apr 01 '24 14:04 ElaadF

Commit modified

ElaadF avatar Apr 01 '24 14:04 ElaadF

Cool, looks promising!

Could you add tests, if possible :

  • at least at api level with yaml request / query output)
  • if possible at service unit test level too, so that it's easier to have an anti regression specification

fanf avatar Apr 02 '24 20:04 fanf

PR updated with a new commit

ElaadF avatar Apr 03 '24 21:04 ElaadF

PR updated with a new commit

ElaadF avatar Apr 04 '24 21:04 ElaadF

Commit modified

ElaadF avatar Apr 04 '24 21:04 ElaadF

PR updated with a new commit

ElaadF avatar Apr 05 '24 00:04 ElaadF

PR updated with a new commit

ElaadF avatar Apr 05 '24 21:04 ElaadF

PR updated with a new commit

ElaadF avatar Apr 06 '24 01:04 ElaadF

PR rebased

ElaadF avatar Apr 06 '24 01:04 ElaadF

PR rebased

ElaadF avatar Apr 06 '24 01:04 ElaadF

PR rebased

ElaadF avatar Jul 22 '24 12:07 ElaadF

PR rebased

ElaadF avatar Jul 22 '24 12:07 ElaadF

PR replaced by https://github.com/Normation/rudder/pull/5779

ElaadF avatar Jul 22 '24 16:07 ElaadF