Add support for storing filters in dashboard entries

[spl3g]

closes #102
This PR adds support for storing filters and changing them in the dashboard entry form.
It adds 2 new tables for storing filters and a component for choosing them.

I wanted to make it so that the Downshift opens again if the tag is not fully finished, but I could not find a way to do this.
I am open to any suggestions regarding my code, and I haven't written any tests yet.. That's why it's a draft PR.

[jmattheis]

Any reason not to use the existing selector which is used for the timespans (ui/src/tag/TagSelector)?

[spl3g]

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.
Also I tweaked suggestions a bit (reversed them) so that they will suit the selection more, instead of creation.
Now, that I think about it more, I shouldn't reverse them, but just remove the first entry (user input) and maybe put it at the end.

[spl3g]

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];
     }

[jmattheis]

Yeah, maybe call it can get a name that's describes what it does. E.g. includeInputValueOnNoMatch

[jmattheis]

I'd rather use the existing picker as it has better UX.

[jmattheis]

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>
    );
};

[spl3g]

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.
It's a bit strange when an include tags selection field offers you to create a tag:

Or prevents you from adding two identical tag names:

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.

[jmattheis]

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.

[spl3g]

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.
From my side, i think, this should be changed:

1. Add a flag to disable tag creation
2. Adjust the margins on TagsSelect or TagKeySelect so they have the same gap size
3. Maybe add a flag to add an X at the end?

[jmattheis]

1. Yeah 2. yeah, it should be consistent. 3. no, improving the existing TagSelector can be done in a separate PR.

[jmattheis]

Currently, this branch doesn't compile:

$ yarn tsc
yarn run v1.22.22
$ /home/jm/src/traggo/server/ui/node_modules/.bin/tsc
src/tag/TagFilterSelector.tsx:99:13 - error TS2322: Type 'void' is not assignable to type '((selectedItem: any, stateAndHelpers: ControllerStateAndHelpers<any>) => void) | undefined'.

99             onChange={handleChange(item, state)}
               ~~~~~~~~

  node_modules/downshift/typings/index.d.ts:40:3
    40   onChange?: (
         ~~~~~~~~
    The expected type comes from property 'onChange' which is declared here on type 'IntrinsicAttributes & IntrinsicClassAttributes<Component<DownshiftProps<any>, any, any>> & Readonly<...> & Readonly<...>'

src/tag/TagFilterSelector.tsx:99:36 - error TS2304: Cannot find name 'item'.

99             onChange={handleChange(item, state)}
                                      ~~~~

src/tag/TagFilterSelector.tsx:99:42 - error TS2304: Cannot find name 'state'.

99             onChange={handleChange(item, state)}
                                            ~~~~~


Found 3 errors.

error Command failed with exit code 1.

[spl3g]

Oh, my bad. I don't know, how i missed that

[jmattheis]

FYI: There is a test failure and two unresolved conversations.

[spl3g]

About the test still failing. The problem is with the sqlite hanging when trying to start a transaction. Removing the SetMaxOpenConns(1) fixes that, but i think that would break something else:

if dialect == "sqlite3" {
	// We use the database connection inside the handlers from the http
	// framework, therefore concurrent access occurs. Sqlite cannot handle
	// concurrent writes, so we limit sqlite to one connection.
	// see https://github.com/mattn/go-sqlite3/issues/274
	// db.DB().SetMaxOpenConns(1)
	db.Exec("PRAGMA foreign_keys = ON")
}

[jmattheis]

The deadlocking test is likely fixed by using tx instead of r.DB here, as there may be only one connection and with the transaction open, r.DB cannot open a new connection.

if _, err := util.FindDashboardRange(tx, *stats.RangeID); err != nil {

[spl3g]

Oh, i missed that