Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Catalogs view should filter by site #801 #804

Merged
merged 1 commit into from
Nov 30, 2023
Merged

Catalogs view should filter by site #801 #804

merged 1 commit into from
Nov 30, 2023

Conversation

CalamityC
Copy link
Collaborator

@CalamityC CalamityC commented Oct 31, 2023 •
edited
Loading

Description

  • set storeid cookie in management views.py
  • clear localStorage in new session
  • filter by currentSite when catalogs page first loads

Related issue: #801

Motivation and Context

The catalog overview shows all contents of the instance by default, but should filter automatically by site
Filter necessary especially when viewing all questions

How has this been tested?

Locally, manually in the UI

Screenshots (if appropriate)

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Checklist

  • I have read the contributor guide.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@CalamityC CalamityC changed the base branch from master to dev-2.1.0 October 31, 2023 16:11
@CalamityC CalamityC marked this pull request as ready for review October 31, 2023 16:48
@CalamityC CalamityC requested a review from jochenklar October 31, 2023 16:48
@MyPyDavid MyPyDavid linked an issue Nov 2, 2023 that may be closed by this pull request
Catalogs view should filter by site #801
Closed
@jochenklar jochenklar added this to the 2.1.0 milestone Nov 16, 2023
jochenklar
jochenklar reviewed Nov 16, 2023
View reviewed changes
rdmo/management/assets/js/store/configureStore.js
@@ -28,7 +39,6 @@ export default function configureStore() {
// load: restore the config from the local storage
const updateConfigFromLocalStorage = () => {
const ls = {...localStorage}

Object.entries(ls).forEach(([lsPath, lsValue]) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add if (lsPath.startsWith('rdmo.management.config.')) { after this line. This was an oversight by me. Otherwise rdmo.storeid and everything else in the store is picked up as well and stored again unter e.g. rdmo.management.config.rdmo.storeid.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ist erledigt. Schaust du bitte noch mal ob ich das if-Statement auch an der richtigen Stelle beende?
Line 57 nach store.dispatch(...

jochenklar
jochenklar requested changes Nov 16, 2023
View reviewed changes
Copy link
Member

@jochenklar jochenklar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see inline comment.

@MyPyDavid MyPyDavid force-pushed the catalogs_view_filter branch from acb741a to 3582b5a Compare November 24, 2023 10:24
@MyPyDavid
Copy link
Member

I've newly rebased the branch onto dev-2.1.0

@MyPyDavid
Copy link
Member

oh no, our e2e tests failed again, I will check it...

@MyPyDavid MyPyDavid added javascript Pull requests that update Javascript code type:feature-request status:work-in-progress labels Nov 24, 2023
@MyPyDavid MyPyDavid force-pushed the catalogs_view_filter branch from 3582b5a to 917d9db Compare November 24, 2023 12:47
@MyPyDavid
Copy link
Member

MyPyDavid commented Nov 24, 2023 •
edited
Loading

makes actually sense that the e2e fails here, in the test we compare the amount of catalogs with Catalog.objects.count() (=4) and now they are actually filtered by this feature (=2).

we have our first use-case of the e2e test @afuetterer! ;)

PS oh no, that would be only for when multisite is True...

@CalamityC @MyPyDavid
Catalogs view should filter by site rdmorganiser#801
4b85c48 
* set storeid cookie in management views.py
* clear localStorage in new session
* when multisite: filter by currentSite when catalogs page first loads
@MyPyDavid MyPyDavid force-pushed the catalogs_view_filter branch from 917d9db to 4b85c48 Compare November 24, 2023 13:31
@MyPyDavid
Copy link
Member

it had to with the setting multisite, the filter does not need to be pre-set when multisite is false..
Ive fixed the commit and tests should pass now.

@CalamityC CalamityC merged commit 0c38d52 into rdmorganiser:dev-2.1.0 Nov 30, 2023
12 checks passed
@CalamityC CalamityC deleted the catalogs_view_filter branch November 30, 2023 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jochenklar jochenklar jochenklar approved these changes

Assignees
No one assigned
Labels
javascript Pull requests that update Javascript code status:work-in-progress type:feature-request
Projects
None yet
Milestone
2.1.0
Development

Successfully merging this pull request may close these issues.

Catalogs view should filter by site
3 participants
@CalamityC @MyPyDavid @jochenklar