Persist dashboard filters in query params #3406

DeNic0la · 2023-04-09T09:00:09Z

The task of Dashboard improvements #3155
"Persist the filters in the URL, for sharing filtered lists and so that when navigating to an activity and back, the same filters are restored"

Dashboard component stores Category and Responsible in Url params.

Open Questions:
The filter values are stored in the /categories/330a052f5d2d - format, is that okay or is there a better Key?

What is the filter value period and how is it written read? I found no way to set that in the UI.

pmattmann · 2023-04-09T09:35:37Z

Dashboard component stores Category and Responsible in Url params.

Open Questions: The filter values are stored in the /categories/330a052f5d2d - format, is that okay or is there a better Key?

What is the filter value period and how is it written read? I found no way to set that in the UI.

Filter values
In the backend it would certainly be your suggested format /categories/330a052f5d2d.
In the frontend, in user-facing URLs (which appear in the browser's address bar), usually we use the ID only.
Example: https://app.ecamp3.ch/camps/882ef43cb421/SoLa/dashboard

Could be something like:

?categories[0]=330a052f5d2d&categories[1]882ef43cb421
?categories[]=330a052f5d2d&categories[]882ef43cb421
?categories=330a052f5d2d,882ef43cb421

Period
The Period-Filter is only displayed, if you have multiple periods (sub-camps, e.g. Vorweekend, Hauptlager, Nach-Tag etc.).
Go to "Admin" and add a second Period.

DeNic0la · 2023-04-13T17:38:45Z

Filter values In the backend it would certainly be your suggested format /categories/330a052f5d2d. In the frontend, in user-facing URLs (which appear in the browser's address bar), usually we use the ID only.

added Helper as Mixin (frontend/src/mixins/formatHalHelperMixin.js) to convert uri to id
(not sure if a mixin is the way to go)

Could be something like: [...]

I used the Vue-router default implementation, its similar so i hope that's fine

Period [...]

Did that

I added a Test for the functionality I added, doesn't test the entire class tho.

usu · 2023-05-01T22:50:42Z

added Helper as Mixin (frontend/src/mixins/formatHalHelperMixin.js) to convert uri to id
(not sure if a mixin is the way to go)

No need for this to be a mixin. You can implement this has a simple helper class and then just export the 2 functions (see frontend/src/helpers/vCalendarDragAndDrop.js for example).

Mixin is only needed, if you need access to the Vue component context.

usu

Generally, looks good to me.

Refactor mixin to helper would be nice (see my other comment)
Any chance we can improve the error handling in beforeMount, when invalid/outdated data is presented (e.g. an ID is presented that was deleted in the meantime)? In my opinion we could silently ignore such data. At the moment, the console spits out some errors and the Select is not shown due to the errors.

…in-url-ecamp#3155

validate query params before writing them to used filter values update unittests add jsdoc typing

update feature branch

github-actions · 2023-06-10T22:37:21Z

⛔ Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

usu

Nice, looks good to me. From my side good to merge.

Don't know why the end-to-end tests fail though. I have triggered a re-run to see if it was just a glitch.

usu · 2023-06-10T22:47:32Z

frontend/src/views/camp/Dashboard.vue

    ])

-    this.loggedInUser = loggedInUser
+    // Once camp data is loaded validate and map values from url param to filter
+    await Promise.all([


Is there a specific reason we need 2 separate calls for Promise.all? Or could we put all the loading promises into one Promise.all and await this one?

Nope, that's leftover code from when I tried some performance optimisation.

update

usu · 2023-06-11T14:14:39Z

Don't know why the end-to-end tests fail though. I have triggered a re-run to see if it was just a glitch.

I found the reason for the failing check and could reproduce locally (see screen recording incl. console output). When I open the dashboard and immediately navigate to a different page (e.g. Print as in the e2e-tests), this triggers a $router.push, but navigation is not immediate (depending on which components need to be loaded; print is one of the pages that takes a bit longer to load).

In parallel, Dashboard is loading data and eventually calls persistRouterState which calls $router.replace. This last calls replaces/aborts the previous $router.push. Hence, the user never arrives at the print page.

Don't have an immediate good solution how to resolve this. Ideas very welcome.

Screen.Recording.2023-06-11.at.09.57.29.mov

DeNic0la · 2023-06-11T17:15:42Z

Sorry i didnt update sooner, i figured it out too but I kinda have some trouble running the project localy. I was working on a way to fix that for some time but I had various problems locally.

usu · 2023-06-12T13:16:26Z

frontend/src/views/camp/Dashboard.vue

+     * Is True until the Component gets unmounted or there is a Navigation to another page
+     */
+    syncUrlQuerActive() {
+      return this.isActive && this.$router.currentRoute.name === 'camp/dashboard'


This didn't work for me. this.$router.currentRoute still equals to the camp/dashboard route, even after the Print button was clicked. It seems that currentRoute is only updated, once the navigation is completely finished.

What we probably would need is to query for a pending navigation route. In vue-router there's no built-in capability to do this. It seems to be implementable with Navigation Guards, however, might not be trivial to catch all the edge cases (see vuejs/vue-router#2012 (comment)).

I see your point and used window.history.replaceState - this doesn't overwrite pending navigations.
This could cause problems with mobile devices tho.

# Conflicts: # frontend/src/views/camp/Dashboard.vue

integrate progress labels

DeNic0la · 2023-08-06T14:35:05Z

It's all done and tested

The failing check is an API Lint - I have no idea why it's failing but it's optional so I should be good?

A Review would be appreciated @carlobeltrame or @pmattmann
Maybe you want to redo your review @usu

BacLuc · 2023-08-07T10:05:30Z

It's all done and tested

The failing check is an API Lint - I have no idea why it's failing but it's optional so I should be good?

A Review would be appreciated @carlobeltrame or @pmattmann Maybe you want to redo your review @usu

Yes, the psalm linter fails since a little more than a week.
The fixes are already here (#3638 and #3672 ), they just need review

usu

Nice, looks good to me. Thanks for the good test coverage 👍

usu · 2023-08-14T07:30:13Z

frontend/src/views/camp/Dashboard.vue

+    persistRouterState() {
+      const query = transformValuesToHalId(this.filter)
+      if (filterAndQueryAreEqual(query, this.$route.query)) return
+      this.$router.replace({ query }).catch((err) => console.warn(err))


So you got it running in the end with $router.replace without the need to use window.history?
Cool. What was the secret trick to make it work?

the fix is on line 394, I implemented an equals check that doesn't trigger as easily. The issue is that the filter contains values such as null or [] which should be equal to undefined in the route.query but the check before did not take that into account. Also the route.query doesn't provide an array if there is only one value. The window.history fix just doesn't refresh navigation. But now it may refresh navigation because it's only called when there is a real value change.

Ah I see. Thanks for clarifying

persist dashboard filters in query params

30477fa

DeNic0la added 3 commits April 9, 2023 14:41

show only id in url params and add period

f46a437

add Dashboard test

1f8a558

add Unittests for implemented functionality

5d35136

DeNic0la marked this pull request as ready for review April 13, 2023 17:30

usu reviewed May 1, 2023

View reviewed changes

DeNic0la added 3 commits June 5, 2023 01:47

Merge remote-tracking branch 'original/devel' into dashboard-filters-…

1e4a2d0

…in-url-ecamp#3155

move formatting utilities to helper

a40967f

validate query params before writing them to used filter values update unittests add jsdoc typing

fix jsdoc

5acb42d

DeNic0la requested a review from usu June 5, 2023 03:28

DeNic0la and others added 2 commits June 5, 2023 17:19

inline variable

d7f9eed

Merge pull request #1 from DeNic0la/devel

0daa6b7

update feature branch

usu added the deploy! Creates a feature branch deployment for this PR label Jun 10, 2023

usu approved these changes Jun 10, 2023

View reviewed changes

usu requested a review from carlobeltrame June 10, 2023 22:55

Merge pull request #2 from DeNic0la/devel

cc76582

update

fix: dont redirect after unload

e57d0cc

DeNic0la added 2 commits June 11, 2023 19:24

remove debug info and wrap all promises into one

501d962

update unittests

17ecc42

DeNic0la changed the title ~~persist dashboard filters in query params~~ [WIP] persist dashboard filters in query params Jun 11, 2023

DeNic0la added 3 commits June 11, 2023 19:44

minor fix

27857a9

minor fix

1ed96ba

simplify and lint

aecb048

usu reviewed Jun 12, 2023

View reviewed changes

DeNic0la added 2 commits August 1, 2023 15:41

Merge branch 'devel' into dashboard-filters-in-url-ecamp#3155

14b7788

# Conflicts: # frontend/src/views/camp/Dashboard.vue

use window history to persist query params

0536fdd

integrate progress labels

DeNic0la changed the title ~~[WIP] persist dashboard filters in query params~~ Persist dashboard filters in query params Aug 1, 2023

DeNic0la temporarily deployed to feature-branch August 1, 2023 15:47 — with GitHub Actions Inactive

improve querySyncHelper and add unit tests

0663adf

DeNic0la temporarily deployed to feature-branch August 2, 2023 20:18 — with GitHub Actions Inactive

refactor and add tests

ede3e68

DeNic0la temporarily deployed to feature-branch August 3, 2023 00:14 — with GitHub Actions Inactive

refactor and add tests

9719413

DeNic0la temporarily deployed to feature-branch August 3, 2023 00:16 — with GitHub Actions Inactive

minor fix

fbc0250

DeNic0la temporarily deployed to feature-branch August 3, 2023 00:37 — with GitHub Actions Inactive

Add Cypress-Tests and refactor

46d4a10

DeNic0la temporarily deployed to feature-branch August 6, 2023 13:59 — with GitHub Actions Inactive

DeNic0la added 2 commits August 6, 2023 16:12

Lint and fix

80d1eb3

Merge branch 'devel' into dashboard-filters-in-url-ecamp#3155

00938d1

DeNic0la temporarily deployed to feature-branch August 6, 2023 14:20 — with GitHub Actions Inactive

fix

8aee5d2

DeNic0la temporarily deployed to feature-branch August 6, 2023 14:24 — with GitHub Actions Inactive

DeNic0la mentioned this pull request Aug 9, 2023

Display responsibilities count on teampage #3675

Closed

usu approved these changes Aug 14, 2023

View reviewed changes

usu requested a review from manuelmeister August 14, 2023 07:30

pmattmann approved these changes Aug 15, 2023

View reviewed changes

pmattmann merged commit 2b22502 into ecamp:devel Aug 15, 2023
20 of 21 checks passed

carlobeltrame mentioned this pull request Aug 15, 2023

Dashboard improvements #3155

Closed

5 tasks

pmattmann mentioned this pull request Aug 15, 2023

fix e2e-prettier #3712

Merged

DeNic0la deleted the dashboard-filters-in-url-#3155 branch August 15, 2023 12:04

DeNic0la mentioned this pull request Aug 20, 2023

api: update dev-data #3701

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persist dashboard filters in query params #3406

Persist dashboard filters in query params #3406

DeNic0la commented Apr 9, 2023 •

edited

Loading

pmattmann commented Apr 9, 2023 •

edited by carlobeltrame

Loading

DeNic0la commented Apr 13, 2023 •

edited

Loading

usu commented May 1, 2023

usu left a comment

github-actions bot commented Jun 10, 2023 •

edited

Loading

usu left a comment

usu Jun 10, 2023

DeNic0la Jun 11, 2023

usu commented Jun 11, 2023

DeNic0la commented Jun 11, 2023

usu Jun 12, 2023

DeNic0la Aug 1, 2023

DeNic0la commented Aug 6, 2023

BacLuc commented Aug 7, 2023

usu left a comment

usu Aug 14, 2023

DeNic0la Aug 15, 2023

usu Aug 15, 2023

Persist dashboard filters in query params #3406

Persist dashboard filters in query params #3406

Conversation

DeNic0la commented Apr 9, 2023 • edited Loading

pmattmann commented Apr 9, 2023 • edited by carlobeltrame Loading

DeNic0la commented Apr 13, 2023 • edited Loading

usu commented May 1, 2023

usu left a comment

Choose a reason for hiding this comment

github-actions bot commented Jun 10, 2023 • edited Loading

⛔ Feature branch deployment currently inactive.

usu left a comment

Choose a reason for hiding this comment

usu Jun 10, 2023

Choose a reason for hiding this comment

DeNic0la Jun 11, 2023

Choose a reason for hiding this comment

usu commented Jun 11, 2023

DeNic0la commented Jun 11, 2023

usu Jun 12, 2023

Choose a reason for hiding this comment

DeNic0la Aug 1, 2023

Choose a reason for hiding this comment

DeNic0la commented Aug 6, 2023

BacLuc commented Aug 7, 2023

usu left a comment

Choose a reason for hiding this comment

usu Aug 14, 2023

Choose a reason for hiding this comment

DeNic0la Aug 15, 2023

Choose a reason for hiding this comment

usu Aug 15, 2023

Choose a reason for hiding this comment

DeNic0la commented Apr 9, 2023 •

edited

Loading

pmattmann commented Apr 9, 2023 •

edited by carlobeltrame

Loading

DeNic0la commented Apr 13, 2023 •

edited

Loading

github-actions bot commented Jun 10, 2023 •

edited

Loading