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

825 add toggle to add/remove current site to/from element sites #875

Merged
merged 27 commits into from
Jun 6, 2024
Merged

825 add toggle to add/remove current site to/from element sites #875

merged 27 commits into from
Jun 6, 2024

Conversation

MyPyDavid
Copy link
Member

@MyPyDavid MyPyDavid commented Dec 8, 2023 •
edited
Loading

Description

The new endpoints that add or remove the current site from the sites of an element are:

Django back-end

questions

  • PUT /api/v1/questions/catalogs/{id}/toggle-site/

tasks

  • PUT /api/v1/tasks/tasks/{id}/toggle-site/

views

  • PUT /api/v1/views/views/{id}/toggle-site/

These are defined in a new ViewSetMixin, ElementToggleCurrentSiteViewSetMixin in
rdmo/management/viewsets.py. This mixin has one action toggle-site and only for put is allowed.
It adds or removes the current site from the object, depending on whether is was there already or not (like a toggle).
For permissions it uses a new (rules-based) CanToggleElementCurrentSite permission class.

The mixin class could be simplified by removing the serializer_class and the get_queryset method.
Could we adapt the ElementSucces reducer to not need the serialized object for these actions?
Permissions could by checked right in the action methods by eg. test_rule().

React front-end:

  • new component: ToggleCurrentSiteLink in rdmo/management/assets/js/components/common/Links.js
  • added elementAction to storeElement and storeElementInit
  • the url routing is done in eg. the MultiSiteApi.js storeElement via the action

Related issue: #825

Motivation and Context

How has this been tested?

Screenshots (if appropriate)

Logged in as example-editor.
image

Types of Changes

  • New feature (non-breaking change which adds functionality)

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.

@MyPyDavid MyPyDavid changed the base branch from main to dev-2.2.0 December 8, 2023 17:31
@MyPyDavid
Copy link
Member Author

need to re-do the commits and add the tests but buttons seem to be working like this

@MyPyDavid MyPyDavid self-assigned this Dec 11, 2023
@MyPyDavid MyPyDavid force-pushed the 825-exclude-sites-assignment-from-protection-of-a-catalogue branch from 8b60c37 to 23767c6 Compare December 14, 2023 08:11
@MyPyDavid MyPyDavid changed the title 825 add toggle to add/remove current site to element sites 825 add toggle to add/remove current site to/from element sites Dec 14, 2023
@MyPyDavid MyPyDavid force-pushed the 825-exclude-sites-assignment-from-protection-of-a-catalogue branch from 23767c6 to 395ce52 Compare December 14, 2023 08:45
@MyPyDavid MyPyDavid added this to the 2.2.0 milestone Dec 14, 2023
@MyPyDavid MyPyDavid requested a review from jochenklar December 14, 2023 10:58
@MyPyDavid MyPyDavid marked this pull request as ready for review December 14, 2023 10:58
@MyPyDavid MyPyDavid linked an issue Jan 4, 2024 that may be closed by this pull request
Exclude sites assignment from "protection" of a catalogue #825
Closed
@MyPyDavid MyPyDavid force-pushed the 825-exclude-sites-assignment-from-protection-of-a-catalogue branch 3 times, most recently from 2bd6281 to c3abc7a Compare January 10, 2024 12:58
@MyPyDavid MyPyDavid force-pushed the 825-exclude-sites-assignment-from-protection-of-a-catalogue branch from c3abc7a to 64f1fec Compare January 10, 2024 16:25
jochenklar
jochenklar reviewed Feb 8, 2024
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.

Thanks for the work. I have some minor suggestions.

@MyPyDavid MyPyDavid requested a review from jochenklar February 13, 2024 15:06
jochenklar
jochenklar requested changes May 14, 2024
View reviewed changes
MyPyDavid added 8 commits May 15, 2024 16:16
@MyPyDavid
chore: refactor storeElement signature
2860398 
Signed-off-by: David Wallace <[email protected]>
@MyPyDavid
chore: add action to methods in Api classes and remove MultiSiteApi
25ed018 
Signed-off-by: David Wallace <[email protected]>
@MyPyDavid
chore: update element components with signature for storeElements and…
00d2218 
… rename hasCurrentSite

Signed-off-by: David Wallace <[email protected]>
@MyPyDavid
chore: rename to hasCurrentSite
9d38aa5 
Signed-off-by: David Wallace <[email protected]>
@MyPyDavid
chore: add element-button class to ReadOnlyIcon for margin-left
3f8a311 
Signed-off-by: David Wallace <[email protected]>
@MyPyDavid
chore: only return super in CanToggleElementCurrentSite.has_permission
5998074 
Signed-off-by: David Wallace <[email protected]>
@MyPyDavid
chore: remove unused logger from management/rules
4474043 
Signed-off-by: David Wallace <[email protected]>
@MyPyDavid
chore: update signature for storeElement in edit components
c56f53b 
Signed-off-by: David Wallace <[email protected]>
@MyPyDavid MyPyDavid requested a review from jochenklar May 15, 2024 15:02
@MyPyDavid MyPyDavid force-pushed the 825-exclude-sites-assignment-from-protection-of-a-catalogue branch from 65ef10e to 8757d2c Compare May 16, 2024 08:09
jochenklar
jochenklar requested changes May 17, 2024
View reviewed changes
@MyPyDavid
Copy link
Member Author

MyPyDavid commented May 21, 2024 •
edited
Loading

  • the current site toggle should not be affected by the "locked" state of the element

@MyPyDavid MyPyDavid requested a review from jochenklar May 22, 2024 08:29
@jochenklar
Copy link
Member

Can this be merged?

@MyPyDavid MyPyDavid merged commit b8ee8ec into dev-2.2.0 Jun 6, 2024
17 checks passed
@MyPyDavid MyPyDavid deleted the 825-exclude-sites-assignment-from-protection-of-a-catalogue branch July 18, 2024 10:54
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

@MyPyDavid MyPyDavid

Labels
type:feature-request
Projects
None yet
Milestone
RDMO 2.2.0
Development

Successfully merging this pull request may close these issues.

Exclude sites assignment from "protection" of a catalogue
2 participants
@MyPyDavid @jochenklar