Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Tags for Organizational Management #369

Merged
merged 5 commits into from
Jun 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions browser/flagr-ui/src/components/Flag.vue
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,36 @@
@save="putFlag(flag)"
></markdown-editor>
</el-row>
<el-row style="margin: 10px;">
<h5>
<span style="margin-right: 10px;">Tags</span>
</h5>
</el-row>
<el-row>
<div class="tags-container-inner">
<el-tag v-for="tag in flag.tags"
:key="tag.id"
closable
:type="warning"
@close="deleteTag(tag)">
{{tag.value}}
</el-tag>
<el-autocomplete
class="tag-key-input"
v-if="tagInputVisible"
v-model="newTag.value"
ref="saveTagInput"
size="mini"
:trigger-on-focus="false"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we set trigger-on-focus to true, it helps to standardize tags creation and reduce typos

Copy link
Contributor

@bradphilips bradphilips Jun 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhouzhuojie this was set to false because focus was triggering a save operation which we didn't want, requiring the saveTagInput method have to handle or ignore values that are empty but would still cancel your operation when tabbed away. This conflicts with entry process we're using with the save on enter and cancel.

:fetch-suggestions="queryTags"
@select="createTag"
@keyup.enter.native="createTag"
@keyup.esc.native="cancelCreateTag"
>
</el-autocomplete>
<el-button v-else class="button-new-tag" size="small" @click="showTagInput">+ New Tag</el-button>
</div>
</el-row>
</el-card>
</el-card>

Expand Down Expand Up @@ -691,6 +721,10 @@ const DEFAULT_VARIANT = {
key: "",
};

const DEFAULT_TAG = {
value: "",
};

const DEFAULT_DISTRIBUTION = {
bitmap: "",
variantID: 0,
Expand Down Expand Up @@ -725,7 +759,9 @@ export default {
dialogEditDistributionOpen: false,
dialogCreateSegmentOpen: false,
entityTypes: [],
allTags: [],
allowCreateEntityType: true,
tagInputVisible: false,
flag: {
createdBy: "",
dataRecordsEnabled: false,
Expand All @@ -734,13 +770,15 @@ export default {
enabled: false,
id: 0,
key: "",
tags:[],
segments: [],
updatedAt: "",
variants: [],
notes: "",
},
newSegment: clone(DEFAULT_SEGMENT),
newVariant: clone(DEFAULT_VARIANT),
newTag: clone(DEFAULT_TAG),
selectedSegment: null,
newDistributions: {},
operatorOptions: operators,
Expand Down Expand Up @@ -896,6 +934,60 @@ export default {
this.$message.success("variant updated");
}, handleErr.bind(this));
},
createTag() {
Axios.post(
`${API_URL}/flags/${this.flagId}/tags`,
this.newTag
).then((response) => {
let tag = response.data;
this.newTag = clone(DEFAULT_TAG);
if (!this.flag.tags.map((tag) => tag.value).includes(tag.value)) {
this.flag.tags.push(tag);
this.$message.success("new tag created");
}
this.tagInputVisible = false;
this.loadAllTags();
}, handleErr.bind(this));
},
cancelCreateTag() {
this.newTag = clone(DEFAULT_TAG);
this.tagInputVisible = false;
},
queryTags(queryString, cb) {
let results = this.allTags.filter((tag) => tag.value.toLowerCase().includes(queryString.toLowerCase()));
cb(results);
},
loadAllTags() {
Axios.get(
`${API_URL}/tags`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that even when we delete the tag from across flags, it still shows up in the autocomplete.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually intended for the Flags delete, because it simply removes the reference to the Tag from the Flag and not the actual Tag itself because it could be referenced by another Flag. Let us know if there's another way you'd like to handle this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhouzhuojie, I was talking to @bradphilips and a couple of our other Flagr users, and we wondered if this would be a preferred way to handle this anyway. It would keep us from creating and deleting the same tag over and over if it goes in and out of use.

Brad also mentioned an option to search for any flags with the removed tag and fully delete the tag if there aren't any. I can see an argument for doing that to prevent unused tags from sticking around.

Personally, I like leaving them the way it's currently set up, but I don't think we'd argue if you want to fully delete the tags.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, lgtm

).then((response) => {
let result = response.data;
this.allTags = result;
}, handleErr.bind(this));
},
showTagInput() {
this.tagInputVisible = true;
this.$nextTick(() => {
this.$refs.saveTagInput.$refs.input.focus();
});
},
deleteTag(tag) {
if (
!confirm(
`Are you sure you want to delete tag #${tag.value}`
)
) {
return;
}

Axios.delete(
`${API_URL}/flags/${this.flagId}/tags/${tag.id}`
).then(() => {
this.$message.success("tag deleted");
this.fetchFlag();
this.loadAllTags();
}, handleErr.bind(this));
},
createConstraint(segment) {
Axios.post(
`${API_URL}/flags/${this.flagId}/segments/${segment.id}/constraints`,
Expand Down Expand Up @@ -1016,6 +1108,7 @@ export default {
},
mounted() {
this.fetchFlag();
this.loadAllTags();
},
};
</script>
Expand Down Expand Up @@ -1157,4 +1250,17 @@ ol.constraints-inner {
.save-remove-variant-row {
padding-bottom: 5px;
}

.tag-key-input {
margin: 2.5px;
width: 20%;
}

.tags-container-inner {
margin-bottom: 10px;
}

.button-new-tag {
margin: 2.5px;
}
</style>
23 changes: 17 additions & 6 deletions browser/flagr-ui/src/components/Flags.vue
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,20 @@
v-on:row-click="goToFlag"
style="width: 100%"
>
<el-table-column prop="id" align="center" label="Flag ID" sortable fixed width="100"></el-table-column>
<el-table-column prop="description" label="Description" min-width="380"></el-table-column>
<el-table-column prop="id" align="center" label="Flag ID" sortable fixed width="95"></el-table-column>
<el-table-column prop="description" label="Description" min-width="300"></el-table-column>
<el-table-column prop="tags" label="Tags" min-width="200">
<template scope="scope">
<el-tag v-for="tag in scope.row.tags" :key="tag.id" :type="warning" disable-transitions>{{ tag.value }}</el-tag>
</template>
</el-table-column>
<el-table-column prop="updatedBy" label="Last Updated By" sortable width="200"></el-table-column>
<el-table-column
prop="updatedAt"
label="Updated At (UTC)"
:formatter="datetimeFormatter"
sortable
width="200"
width="165"
></el-table-column>
<el-table-column
prop="enabled"
Expand Down Expand Up @@ -123,9 +128,12 @@ export default {
filteredFlags: function() {
if (this.searchTerm) {
return this.flags.filter(
({ id, description }) =>
id.toString().includes(this.searchTerm) ||
description.toLowerCase().includes(this.searchTerm.toLowerCase())
({ id, description, tags }) =>
this.searchTerm.split(',').map(term => {
return id.toString().includes(term) ||
description.toLowerCase().includes(term.toLowerCase()) ||
tags.map(tag => tag.value.toLowerCase().includes(term.toLowerCase())).includes(true)
}).every((e) => e)
);
}
return this.flags;
Expand Down Expand Up @@ -178,4 +186,7 @@ export default {
border-right-color: #dcdfe6;
}
}
.el-tag {
margin: 2.5px;
}
</style>
Loading