Skip to content

Fix routing bugs in security dashboards

Paul Gascou-Vaillancourt requested to merge 11412-fix-security-dashboard-history-bug into master

Context

In the security dashboard, we sync the store and the router so that filters values are reflected in the URL, and vice-versa. This means that:

  • Whenever the user changes a filter's value, we update the store, then the router
  • Whenever the user navigates backwards, we update the store based on the new URL

This is all handled in a Vuex plugin: sync_with_router.js

We discovered that it's impossible to go back more than one entry in the history: using the back button once works as expected, but subsequent usages seem to have no effect on the filters.

What's happening?

When the user navigates backwards, we trigger some store actions to update the store based on the query string: sync_with_router.js#L17-35 While the store is being updated, we "freeze" the plugin by setting a syncingRouter flag to true.

The store->router update process works by programmatically triggering navigation events when specific mutations are being committed: sync_with_router.js#L38-49.

The plugin currently subscribes to 2 mutations:

  • vulnerabilities/SET_VULNERABILITIES_HISTORY_DAY_RANGE
  • vulnerabilities/RECEIVE_VULNERABILITIES_SUCCESS

The problem here is that vulnerabilities/RECEIVE_VULNERABILITIES_SUCCESS is an async mutation which is committed once vulnerabilities have been successfully fetched, at which point the plugin has been "unfreezed" already (syncingRouter === false). This means that whenever we refresh the vulnerabilities list, an entry is pushed to the browser's history. Thus, when using the back button, the current route ends up being duplicated in the history.

graph LR;
  subgraph store
  setAllFilters --> fetchVulnerabilities --> RECEIVE_VULNERABILITIES_SUCCESS[vulnerabilities/RECEIVE_VULNERABILITIES_SUCCESS];
  end
  subgraph router
  Router[Navigation event] -- queryString --> syncingRouter["syncingRouter: true"] --> setAllFilters;
  notSyncingRouter["syncingRouter: false"];
  Router[Navigation event] --> History[New history entry];
  end
  setAllFilters --> notSyncingRouter;
  RECEIVE_VULNERABILITIES_SUCCESS[vulnerabilities/RECEIVE_VULNERABILITIES_SUCCESS] -- state --> Router

What does this MR do?

This MR drops store.subscribe() in favor of store.subscribeAction() in order to hook into vulnerabilities/fetchVulnerabilities and vulnerabilities/setVulnerabilitiesHistoryDayRange actions instead of low-level mutations. This lets us perform store->router updates more reliably without polluting the browser's history because we process filters changes synchronously. This approach has another benefit in terms of UX: URL updates happen immediately after filters change as opposed to the previous approach where URL updates would be delayed until the XHR query for retrieving vulnerabilities resolved.

One more thing...

This MR fixes another bug where the Project Security Dashboard would not init the filters based on the query string. Instead, the store would initialize the filters with their default values and call router.push() to remove custom filter values from the query string. This was due to the fact that this dashboard was initialized without a router and thus ignored the URL completely until the store triggered some routing event. This has been fixed by properly installing the router in the Project Security Dashboard.

Screenshots

Before After
bad-gsd-history-back-button sd_history

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

Edited by Paul Gascou-Vaillancourt

Merge request reports