Fixes #21119: When a technique is not in the active techique tree, it can't be deleted in editor #4292
Conversation
Fixes #20281: Query seems to be ignored for pendings nodes
…o branches/rudder/6.2
…o branches/rudder/6.2
…o branches/rudder/6.2
…o branches/rudder/6.2
…o branches/rudder/6.2
ElaadF
added
qa: Can't merge
WIP
ElaadF
force-pushed
the
bug_21119/when_a_technique_is_not_in_the_active_techique_tree_it_can_t_be_deleted_in_editor
branch
4 times, most recently
from
c2809cc
to
9d12411
Compare
ElaadF
removed
qa: Can't merge
WIP
ElaadF
force-pushed
the
bug_21119/when_a_technique_is_not_in_the_active_techique_tree_it_can_t_be_deleted_in_editor
branch
from
9d12411
to
2bb7648
Compare
| @@ -138,40 +134,56 @@ class TechniqueWriter ( | |||
| cr1.copy(directives = cr1.directives ++ cr2.directives) | |||
| } | |||
|
|
|||
| for { | |||
| directives <- readDirectives.getFullDirectiveLibrary().map(_.allActiveTechniques.values.filter(_.techniqueName.value == techniqueId.name.value).flatMap(_.directives).filter(_.techniqueVersion == techniqueId.version)) | |||
| technique <- techniqueRepository.get(techniqueId).notOptional(s"No Technique with ID '${techniqueId.toString()}' found in reference library.") | |||
There was a problem hiding this comment.
You should extract the for loop in a function (maybe an inner function, it does not need to be global for now)
Same for the none part, it should be in an inner function
| for { | ||
| _ <- ZIO.foreach(unknownTechniquesDir) { f => | ||
| val cat = f.pathAsString.substring(s"${basePath}/techniques/".length).split("/").filter(s => s != techniqueName && s != techniqueVersion).toList | ||
| archiver.deleteTechnique(techniqueName, techniqueVersion, cat, modId, committer, s"Deleting invalid technique ${techniqueName}/${techniqueVersion}").runNow |
There was a problem hiding this comment.
It's forbidden to put runNow into a ZIO effect. You need to deal with the effects, especially with the error case: if the archiver doesn't succeed, what needs to be done ? (something like delete file by hand ? Or perhaps give a procedure in logs for an admin: delete that, commit in git, reload techniques)
ElaadF
added
qa: Can't merge
WIP
|
You should split both branch of your code in inner function, it's complicated to understand what both branch is doing. Have you not seen my previous comment ? https://github.com/Normation/rudder/pull/4292/files#r891297849 |
|
I'm surprised I did refactor in functions, I had pushed the changes but didn't appear.. I will check locally |
| } | ||
| } | ||
|
|
||
| def removeInvalidTechnique(basePath: String, techniqueName: String): IOResult[Unit] = { |
There was a problem hiding this comment.
@VinceMacBuche the inner function
VinceMacBuche
dismissed
fanf’s stale review
Dismissing review, changes were made
|
This PR is not mergeable to upper versions. |
…o branches/rudder/6.2
|
OK, squash merging this PR |
… can't be deleted in editor
ElaadF
force-pushed
the
bug_21119/when_a_technique_is_not_in_the_active_techique_tree_it_can_t_be_deleted_in_editor
branch
from
55b310b
to
c773656
Compare
https://issues.rudder.io/issues/21119