canonical / test_observer

Flutter based dashboard for visualising SRU regression test results
2 stars 2 forks source link

Artefact filters in url query parameters #139

Closed omar-selo closed 3 months ago

omar-selo commented 3 months ago

Resolves RTW-243.

Changes:

Video: Screencast from 2024-03-15 16-20-54.webm

omar-selo commented 3 months ago

@andrejvelichkovski that comment makes it clear to me that you've really dug deep into this PR. Thanks, I really appreciate that! I will try to explain it thoroughly as this behavior isn't accidental.

As you mentioned the edge case happens when the url has an option that's not present in any of the artefacts we've fetched from the backend. It's important to note that, this option could be valid, but it just so happens that none of the artefacts we got from the backend had it. So the question is what should we do in that case.

  1. Ignore it. This works well if the option was invalid. If the option is valid however (e.g. Assignee=Andrej+Velichkovski) then ignoring will produce inconsistent results depending on whether the option was present in some artefacts or not. i.e. you would either see all artefacts or your artefacts depending on what artefacts are present. Even though you didn't change the url (you had it copied for instance).
  2. Just apply it. This can cause an issue where you have a filter applied but you don't see that a filter is applied on the side.
  3. Apply it and add it to the side filters. This way you don't have the issue from 2 and you will get consistent results with the same url (regardless of whether there are artefacts that have this option or not). The downside here is that you could add filters that don't exist. It can be annoying if it was a misspelling as it would appear as if this is a valid filter.

All of the above have issues. But the question goes down to either 1 or 3. It is also possible to add a request to backend to ask what are all the valid options. It would solve these issues as then we can only show this filter if it was actually valid. But I feel that it would add unnecessary complexity. I personally am a bit more inclined towards 3 than 1 because a user will likely copy a url instead of construct it themselves, and in that case the option shouldn't be misspelled or incorrect. But I do see the problem with it. wdyt?

andrejvelichkovski commented 3 months ago

@andrejvelichkovski that comment makes it clear to me that you've really dug deep into this PR. Thanks, I really appreciate that! I will try to explain it thoroughly as this behavior isn't accidental.

As you mentioned the edge case happens when the url has an option that's not present in any of the artefacts we've fetched from the backend. It's important to note that, this option could be valid, but it just so happens that none of the artefacts we got from the backend had it. So the question is what should we do in that case.

1. Ignore it. This works well if the option was invalid. If the option is valid however (e.g. `Assignee=Andrej+Velichkovski`) then ignoring will produce inconsistent results depending on whether the option was present in some artefacts or not. i.e. you would either see all artefacts or your artefacts depending on what artefacts are present. Even though you didn't change the url (you had it copied for instance).

2. Just apply it. This can cause an issue where you have a filter applied but you don't see that a filter is applied on the side.

3. Apply it and add it to the side filters. This way you don't have the issue from 2 and you will get consistent results with the same url (regardless of whether there are artefacts that have this option or not). The downside here is that you could add filters that don't exist. It can be annoying if it was a misspelling as it would appear as if this is a valid filter.

All of the above have issues. But the question goes down to either 1 or 3. It is also possible to add a request to backend to ask what are all the valid options. It would solve these issues as then we can only show this filter if it was actually valid. But I feel that it would add unnecessary complexity. I personally am a bit more inclined towards 3 than 1 because a user will likely copy a url instead of construct it themselves, and in that case the option shouldn't be misspelled or incorrect. But I do see the problem with it. wdyt?

Oh, I now understand the full scope of the issue. I guess we have a set of "Available Filters", for example all users that can get assigned to an artefact, these are defined in our database, then we have a set of "Detected filters" which in the context of users are all users that are detected to be assigned to at least on one artefact, this is detected on the frontend.

I didn't think of the fact that in the URL a user might specify for a filter that is "Available" but not "Detected".

I'm also more inclined towards approach 3 you described. Most likely the users will not specify filters in URLs themselves, and even if they do it, when they see their filter appear, they will be "expecting" to see it.

omar-selo commented 3 months ago

Ah I like the wording "Detected Filters", I'll update the PR to use it