-
-
Notifications
You must be signed in to change notification settings - Fork 81
Add support for storing filters in dashboard entries #215
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
base: master
Are you sure you want to change the base?
Changes from 4 commits
f25e5e3
dcd034b
3d09ce9
09d830f
ddc80fb
2a0474d
97969f4
da667eb
25a7a7e
a6cf516
af5e6b2
7508f5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| package convert | ||
|
|
||
| import ( | ||
| "github.com/traggo/server/generated/gqlmodel" | ||
| "github.com/traggo/server/model" | ||
| ) | ||
|
|
||
| func excludedTagsToExternal(tags []model.DashboardExcludedTag) []*gqlmodel.TimeSpanTag { | ||
| result := []*gqlmodel.TimeSpanTag{} | ||
| for _, tag := range tags { | ||
| result = append(result, &gqlmodel.TimeSpanTag{ | ||
| Key: tag.Key, | ||
| Value: tag.StringValue, | ||
| }) | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| func ExcludedTagsToInternal(gqls []*gqlmodel.InputTimeSpanTag) []model.DashboardExcludedTag { | ||
| result := make([]model.DashboardExcludedTag, 0) | ||
| for _, tag := range gqls { | ||
| result = append(result, excludedTagToInternal(*tag)) | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| func excludedTagToInternal(gqls gqlmodel.InputTimeSpanTag) model.DashboardExcludedTag { | ||
| return model.DashboardExcludedTag{ | ||
| Key: gqls.Key, | ||
| StringValue: gqls.Value, | ||
| } | ||
| } | ||
|
|
||
| func includedTagsToExternal(tags []model.DashboardIncludedTag) []*gqlmodel.TimeSpanTag { | ||
| result := []*gqlmodel.TimeSpanTag{} | ||
| for _, tag := range tags { | ||
| result = append(result, &gqlmodel.TimeSpanTag{ | ||
| Key: tag.Key, | ||
| Value: tag.StringValue, | ||
| }) | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| func IncludedTagsToInternal(gqls []*gqlmodel.InputTimeSpanTag) []model.DashboardIncludedTag { | ||
| result := make([]model.DashboardIncludedTag, 0) | ||
| for _, tag := range gqls { | ||
| result = append(result, includedTagToInternal(*tag)) | ||
| } | ||
| return result | ||
| } | ||
|
|
||
| func includedTagToInternal(gqls gqlmodel.InputTimeSpanTag) model.DashboardIncludedTag { | ||
| return model.DashboardIncludedTag{ | ||
| Key: gqls.Key, | ||
| StringValue: gqls.Value, | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| package entry | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/jinzhu/gorm" | ||
| "github.com/traggo/server/generated/gqlmodel" | ||
| "github.com/traggo/server/model" | ||
| ) | ||
|
|
||
| func tagsDuplicates(src []*gqlmodel.InputTimeSpanTag, dst []*gqlmodel.InputTimeSpanTag) *gqlmodel.InputTimeSpanTag { | ||
| existingTags := make(map[string]struct{}) | ||
| for _, tag := range src { | ||
| existingTags[tag.Key+":"+tag.Value] = struct{}{} | ||
| } | ||
|
|
||
| for _, tag := range dst { | ||
| if _, ok := existingTags[tag.Key+":"+tag.Value]; ok { | ||
| return tag | ||
| } | ||
|
|
||
| existingTags[tag.Key] = struct{}{} | ||
| } | ||
spl3g marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func tagsExist(db *gorm.DB, userID int, tags []*gqlmodel.InputTimeSpanTag) error { | ||
| existingTags := make(map[string]struct{}) | ||
|
|
||
| for _, tag := range tags { | ||
| if _, ok := existingTags[tag.Key]; ok { | ||
| return fmt.Errorf("tag '%s' is present multiple times", tag.Key) | ||
spl3g marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| if db.Where("key = ?", tag.Key).Where("user_id = ?", userID).Find(new(model.TagDefinition)).RecordNotFound() { | ||
| return fmt.Errorf("tag '%s' does not exist", tag.Key) | ||
| } | ||
|
|
||
| existingTags[tag.Key] = struct{}{} | ||
| } | ||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import InputLabel from '@material-ui/core/InputLabel'; | |
| import Select from '@material-ui/core/NativeSelect/NativeSelect'; | ||
| import {EntryType, StatsInterval} from '../../gql/__generated__/globalTypes'; | ||
| import {TagKeySelector} from '../../tag/TagKeySelector'; | ||
| import {TagFilterSelector} from '../../tag/TagFilterSelector'; | ||
| import {Dashboards_dashboards_items, Dashboards_dashboards_items_statsSelection_range} from '../../gql/__generated__/Dashboards'; | ||
| import {RelativeDateTimeSelector} from '../../common/RelativeDateTimeSelector'; | ||
| import {parseRelativeTime} from '../../utils/time'; | ||
|
|
@@ -31,6 +32,27 @@ export const isValidDashboardEntry = (item: Dashboards_dashboards_items): boolea | |
| export const DashboardEntryForm: React.FC<EditPopupProps> = ({entry, onChange: setEntry, disabled = false, ranges}) => { | ||
| const [staticRange, setStaticRange] = React.useState(!entry.statsSelection.rangeId); | ||
|
|
||
| const [excludeTags, setExcludeTags] = React.useState( | ||
| (entry.statsSelection.excludeTags || []).map((tag) => ({ | ||
| tag: { | ||
| key: tag.key, | ||
| color: '', | ||
| __typename: 'TagDefinition' as 'TagDefinition', | ||
| }, | ||
| value: tag.value, | ||
| })) | ||
| ); | ||
| const [includeTags, setIncludeTags] = React.useState( | ||
| (entry.statsSelection.includeTags || []).map((tag) => ({ | ||
| tag: { | ||
| key: tag.key, | ||
| color: '', | ||
| __typename: 'TagDefinition' as 'TagDefinition', | ||
| }, | ||
| value: tag.value, | ||
| })) | ||
| ); | ||
spl3g marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| const range: Dashboards_dashboards_items_statsSelection_range = entry.statsSelection.range | ||
| ? entry.statsSelection.range | ||
| : { | ||
|
|
@@ -189,6 +211,32 @@ export const DashboardEntryForm: React.FC<EditPopupProps> = ({entry, onChange: s | |
| setEntry(entry); | ||
| }} | ||
| /> | ||
| <TagFilterSelector | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason not to use the existing selector which is used for the timespans (ui/src/tag/TagSelector)?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted for it to look more like TagKeySelector, to be more precise, the chips there don't have color and there is an X at the end.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be okay if i changed useSuggest a little? diff --git a/ui/src/tag/suggest.ts b/ui/src/tag/suggest.ts
index 31a6f82..450b220 100644
--- a/ui/src/tag/suggest.ts
+++ b/ui/src/tag/suggest.ts
@@ -9,7 +9,8 @@ export const useSuggest = (
tagResult: QueryResult<Tags, {}>,
inputValue: string,
usedTags: string[],
- skipValue = false
+ skipValue = false,
+ showTagValue = true
): TagSelectorEntry[] => {
const [tagKeySomeCase, tagValue] = inputValue.split(':');
const tagKey = tagKeySomeCase.toLowerCase();
@@ -22,7 +23,7 @@ export const useSuggest = (
});
if (exactMatch && tagValue !== undefined && usedTags.indexOf(exactMatch.key) === -1 && !skipValue) {
- return suggestTagValue(exactMatch, tagValue, valueResult);
+ return suggestTagValue(exactMatch, tagValue, valueResult, showTagValue);
} else {
return suggestTag(exactMatch, tagResult, tagKey, usedTags);
}
@@ -59,11 +60,12 @@ const suggestTag = (
const suggestTagValue = (
exactMatch: TagSelectorEntry['tag'],
tagValue: string,
- valueResult: QueryResult<SuggestTagValue, SuggestTagValueVariables>
+ valueResult: QueryResult<SuggestTagValue, SuggestTagValueVariables>,
+ showTagValue: boolean
): TagSelectorEntry[] => {
let someValues = (valueResult.data && valueResult.data.values) || [];
- if (someValues.indexOf(tagValue) === -1) {
+ if (showTagValue && someValues.indexOf(tagValue) === -1) {
someValues = [tagValue, ...someValues];
}
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, maybe call it can get a name that's describes what it does. E.g. includeInputValueOnNoMatch
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather use the existing picker as it has better UX.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I'd say changing the style of the existing selector is much easier than reimplementing it. E.g. this makes it look pretty much the same as the other inputs in the form. const StyledTagSelector = ({label, ...props}: TagSelectorProps & {label: string}) => {
return (
<Box mt={1}>
<FormControl fullWidth>
<Box pt={2}>
<InputLabel shrink> {label} </InputLabel>
<Box className="MuiInput-underline">
<TagSelector {...props} />
</Box>
</Box>
</FormControl>
</Box>
);
};
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that using the old selector is easier but i was talking more about the styling and spacing of tag chips. Also i think it is more tailored towards creating and assigning tags to timespans. I understand that more props could be added to TagSelector to change this behavior, but this would add more complexity to the component. I think it would be better to keep it within its current scope.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think there are many changes needed in the TagSelector, probably <10 lines of code. I'll insist on this change, but I can do it myself. It'll probably take some time, as I'm busy right now.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was busy at that time too, but now i can work on this. I can do the changes to the TagsSelect, but first i need to know, what changes do you want.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| type="Exclude" | ||
| value={excludeTags} | ||
| onChange={(tags) => { | ||
| setExcludeTags(tags); | ||
| entry.statsSelection.excludeTags = tags.map((tag) => ({ | ||
| key: tag.tag.key, | ||
| value: tag.value, | ||
| __typename: 'TimeSpanTag', | ||
| })); | ||
| setEntry(entry); | ||
| }} | ||
| /> | ||
| <TagFilterSelector | ||
| type="Include" | ||
| value={includeTags} | ||
| onChange={(tags) => { | ||
| setIncludeTags(tags); | ||
| entry.statsSelection.includeTags = tags.map((tag) => ({ | ||
| key: tag.tag.key, | ||
| value: tag.value, | ||
| __typename: 'TimeSpanTag', | ||
| })); | ||
| setEntry(entry); | ||
| }} | ||
| /> | ||
| </> | ||
| ); | ||
| }; | ||
|
|
||



Uh oh!
There was an error while loading. Please reload this page.