Skip to content

Commit

Permalink
fix(cliArgs): Fixing the CLI args bug for removed overrides
Browse files Browse the repository at this point in the history
When you override template args in new task dialog the isCLIArgsOverridden manages that well, but when you remove a CLI arg in new task dialog since it doesn't exist anymore the iteration doesn't know it and it falls back to template which it does exist there and then gets implemented, to solve this:
- I added a new non-database field to front-end and backend called RemovedArguments
- In the front-end I made a new data binding and when an template declarated argument gets deleted on task dialog it populates it to removed_arguments object
- Then when this get sent to the back-end, then back-end first removes and RemovedArguments from templateExtraArgs, so this prevents falling back to template args and finding it again there
- Then it does the rest same as before to compare and override if there are existing arguments both in template and task with same key but different values.

- Also ArgsPicker was problematic on populating CLI args on task dialog especially for re-runs because it wasn't getting populated correctly when you were trying for few times, and also it wasn't population a "rerun" with whatever arguments was selected there before, so it would make it an actual re-run.
- I also fixed a bug for re-run button on TaskList, word `Re` was getting declared twice as hardcoded prefixed to the word `run`.

- Fixed the api-docs and ran the tests.
  • Loading branch information
Guney Saramali committed Nov 1, 2024
1 parent c9a3492 commit ca83807
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 18 deletions.
10 changes: 10 additions & 0 deletions api-docs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,16 @@ definitions:
type:
- string
- 'null'
arguments:
type:
- string
- 'null'
removed_arguments:
type:
- array
- 'null'
items:
type: string

TaskOutput:
type: object
Expand Down
13 changes: 7 additions & 6 deletions db/Task.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ type Task struct {
Diff bool `db:"diff" json:"diff"`

// override variables
Playbook string `db:"playbook" json:"playbook"`
Environment string `db:"environment" json:"environment"`
Limit *string `db:"hosts_limit" json:"limit"`
Secret string `db:"-" json:"secret"`
Arguments *string `db:"arguments" json:"arguments"`
GitBranch *string `db:"git_branch" json:"git_branch"`
Playbook string `db:"playbook" json:"playbook"`
Environment string `db:"environment" json:"environment"`
Limit *string `db:"hosts_limit" json:"limit"`
Secret string `db:"-" json:"secret"`
Arguments *string `db:"arguments" json:"arguments"`
RemovedArguments []string `db:"-" json:"removed_arguments"`
GitBranch *string `db:"git_branch" json:"git_branch"`

UserID *int `db:"user_id" json:"user_id"`
IntegrationID *int `db:"integration_id" json:"integration_id"`
Expand Down
23 changes: 19 additions & 4 deletions services/tasks/LocalJob.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"maps"
"os"
"regexp"
"slices"

"path"
"strconv"
Expand Down Expand Up @@ -382,24 +383,38 @@ func (t *LocalJob) getPlaybookArgs(username string, incomingVersion *string) (ar
taskExtraArgs = append(taskExtraArgs, "--limit="+*t.Task.Limit)
}

// override the args from task to template
for _, ra := range t.Task.RemovedArguments {
rai := slices.Index(templateExtraArgs, ra)
if rai != -1 {
templateExtraArgs = append(templateExtraArgs[:rai], templateExtraArgs[rai+1:]...)
}
}

for _, taskArg := range taskExtraArgs {
for i, tmplArg := range templateExtraArgs {
for tai, tmplArg := range templateExtraArgs {
ok, err := isCLIArgsOverridden(tmplArg, taskArg)
if err != nil {
t.Log(err.Error())
}

if ok {
t.Log("Overrideing template CLI with " + taskArg)
templateExtraArgs[i] = taskArg
templateExtraArgs = append(templateExtraArgs[:tai], templateExtraArgs[tai+1:]...)
break
}

if slices.Contains(templateExtraArgs, taskArg) {
templateExtraArgs = append(templateExtraArgs[:tai], templateExtraArgs[tai+1:]...)
}

}
}

templateExtraArgs = append(templateExtraArgs, taskExtraArgs...)

args = append(args, templateExtraArgs...)
// args = append(args, taskExtraArgs...) // old implementation
args = append(args, playbookName)
fmt.Println(args)

if line, ok := inputMap[db.AccessKeyRoleAnsibleUser]; ok {
inputs["SSH password:"] = line
Expand Down
2 changes: 2 additions & 0 deletions web/src/components/ArgsPicker.vue
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export default {
watch: {
vars(val) {
this.var = val || [];
this.modifiedVars = (this.var || []).map((v) => ({ name: v }));
},
},
Expand Down Expand Up @@ -155,6 +156,7 @@ export default {
},
deleteVar(index) {
this.$emit('removed', this.modifiedVars[index].name);
this.modifiedVars.splice(index, 1);
this.$emit('change', this.modifiedVars.map((x) => x.name));
},
Expand Down
33 changes: 26 additions & 7 deletions web/src/components/TaskForm.vue
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
v-if="template.allow_override_args_in_task"
:vars="args"
@change="setArgs"
@removed="setRemovedArgs"
/>

</v-form>
Expand Down Expand Up @@ -149,7 +150,7 @@ export default {
},
computed: {
args() {
return JSON.parse(this.item.arguments || this.template.arguments || '[]');
return JSON.parse(this.item.arguments || '[]');
},
},
Expand Down Expand Up @@ -184,6 +185,15 @@ export default {
this.item.arguments = JSON.stringify(args || []);
},
setRemovedArgs(arg) {
console.log(arg);

Check warning on line 189 in web/src/components/TaskForm.vue

View workflow job for this annotation

GitHub Actions / build-local

Unexpected console statement
if (!this.item.removed_arguments) {
this.item.removed_arguments = [];
}
this.item.removed_arguments.push(arg);
},
getTaskMessage(task) {
let buildTask = task;
Expand Down Expand Up @@ -225,7 +235,7 @@ export default {
this.item.secret = JSON.stringify(this.editedSecretEnvironment);
},
async afterLoadData() {
async beforeLoadData() {
this.assignItem(this.sourceTask);
this.item.template_id = this.templateId;
Expand All @@ -234,6 +244,10 @@ export default {
this.item.params = {};
}
if (!this.item.removed_arguments) {
this.item.removed_arguments = [];
}
// this.advancedOptions = this.item.arguments != null;
this.template = (await axios({
Expand All @@ -242,16 +256,21 @@ export default {
responseType: 'json',
})).data;
if (this.item != null) {
this.item.limit = this.template.limit;
this.item.arguments = this.template.arguments;
}
this.buildTasks = this.template.type === 'deploy' ? (await axios({
keys: 'get',
url: `/api/project/${this.projectId}/templates/${this.template.build_template_id}/tasks?status=success`,
responseType: 'json',
})).data.filter((task) => task.status === 'success') : [];
},
afterLoadData() {
if (this.item != null) {
this.setArgs(JSON.parse(this.template.arguments));
this.item.limit = this.template.limit;
}
this.assignItem(this.sourceTask);
this.item.template_id = this.templateId;
if (this.item.build_task_id == null
&& this.buildTasks.length > 0
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/TaskList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<div v-if="tasks != null">
<EditDialog
v-model="newTaskDialog"
:save-button-text="$t('Re' + getActionButtonTitle())"
:save-button-text="$t(getActionButtonTitle())"
@save="onTaskCreated"
>
<template v-slot:title={}>
Expand Down

0 comments on commit ca83807

Please sign in to comment.