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

MyPyDavid · 2023-12-08T17:23:34Z

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.

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 · 2023-12-08T17:32:44Z

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

…nd tests

jochenklar

Thanks for the work. I have some minor suggestions.

rdmo/management/assets/js/components/element/Catalog.js

rdmo/questions/viewsets.py

rdmo/management/assets/js/api/QuestionsApi.js

jochenklar · 2024-02-08T17:15:49Z

rdmo/management/assets/js/components/common/Links.js

+const ToggleCurrentSiteLink = ({ has_current_site, locked, onClick, disabled }) => {
+  const className = classNames({
+    'element-btn-link fa': true,
+    'fa-plus-square-o': !has_current_site,


The icons are not super, but I don't have a better suggestion. Maybe the transition to material symbols will solve this at one point.

ok maybe like this?

I also think it is better when the toogle site link is not the first one.

fa-plus-square and fa-minus-square

rdmo/management/rules.py

rdmo/management/viewsets.py

…ion mixins, makes urls part of api/v1

…ementAction

…ettings.SITE_ID

rdmo/core/permissions.py

rdmo/management/assets/js/components/element/Catalog.js

rdmo/management/assets/js/actions/elementActions.js

rdmo/management/assets/js/api/ViewsApi.js

rdmo/management/assets/js/api/MultiSiteApi.js

rdmo/management/assets/js/components/common/Links.js

jochenklar · 2024-05-14T15:58:51Z

rdmo/management/assets/js/components/common/Links.js

+const ToggleCurrentSiteLink = ({ has_current_site, locked, onClick, disabled }) => {
+  const className = classNames({
+    'element-btn-link fa': true,
+    'fa-plus-square-o': !has_current_site,


ok maybe like this?

I also think it is better when the toogle site link is not the first one.

jochenklar · 2024-05-14T15:59:07Z

rdmo/management/assets/js/components/common/Links.js

+const ToggleCurrentSiteLink = ({ has_current_site, locked, onClick, disabled }) => {
+  const className = classNames({
+    'element-btn-link fa': true,
+    'fa-plus-square-o': !has_current_site,


fa-plus-square and fa-minus-square

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

… rename hasCurrentSite Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

rdmo/management/assets/js/components/element/Question.js

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

jochenklar · 2024-05-17T09:14:39Z

rdmo/core/tests/__init__.py

Can we move this whole file to rdmo/core/tests/utils.py? I am not a big fan of to much indirection in tests (I got that from 2 scoops of django), but I guess it makes sense here. I a bigger test cleanup we could also move some of the constants to this file as well. Maybe a constants.py and a utils.py.

Another idea would be to use fixtures for this (the utils part).

jochenklar · 2024-05-17T09:16:18Z

rdmo/management/assets/js/actions/elementActions.js

  return function(dispatch, getState) {
-    dispatch(storeElementInit(element))
+
+    dispatch(storeElementInit(element, elementAction))


I think elementAction is not needed here.

MyPyDavid changed the base branch from main to dev-2.2.0 December 8, 2023 17:31

MyPyDavid self-assigned this Dec 11, 2023

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 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 force-pushed the 825-exclude-sites-assignment-from-protection-of-a-catalogue branch from 23767c6 to 395ce52 Compare December 14, 2023 08:45

MyPyDavid added the type:feature-request label Dec 14, 2023

MyPyDavid added this to the 2.2.0 milestone Dec 14, 2023

MyPyDavid requested a review from jochenklar December 14, 2023 10:58

MyPyDavid marked this pull request as ready for review December 14, 2023 10:58

jochenklar force-pushed the dev-2.2.0 branch from ca32db7 to d10fa87 Compare December 21, 2023 16:22

MyPyDavid linked an issue Jan 4, 2024 that may be closed by this pull request

Exclude sites assignment from "protection" of a catalogue #825

Open

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

Update version

4e64e34

MyPyDavid force-pushed the dev-2.2.0 branch from d10fa87 to 4e64e34 Compare January 10, 2024 16:24

MyPyDavid added 4 commits January 10, 2024 17:25

tests: add example users to core test_views

e734196

chore: add new CanToggleElementCurrentSite permission class and rules

fb5304e

chore: add ElementToggleCurrentSiteViewSetMixin class

8ff469b

feat: #825 add toggle button for element.sites endpoints components a…

64f1fec

…nd tests

MyPyDavid force-pushed the 825-exclude-sites-assignment-from-protection-of-a-catalogue branch from c3abc7a to 64f1fec Compare January 10, 2024 16:25

MyPyDavid force-pushed the dev-2.2.0 branch from 4e64e34 to 6debf66 Compare January 17, 2024 08:22

jochenklar reviewed Feb 8, 2024

View reviewed changes

MyPyDavid added the status:work-in-progress label Feb 9, 2024

MyPyDavid added 5 commits February 12, 2024 11:46

chore: refactor ElementToggleCurrentSiteViewSetMixin into ViewSet act…

0c01099

…ion mixins, makes urls part of api/v1

chore: refactor endpoint to toggle-site

59ff5fa

js: refactor toggle site put call in MultiSiteApi and storeElement el…

55b8912

…ementAction

js: fix show/hide for ToggleCurrentSiteLink

4b0db2e

js: include toggleCurrentSite action in element components

e4d525c

MyPyDavid added 2 commits February 12, 2024 17:32

js: remove console.log

bf61f0b

chore: rename rule to toggle_site, remove is_element_editor and use s…

b55935c

…ettings.SITE_ID

MyPyDavid requested a review from jochenklar February 13, 2024 15:06

MyPyDavid force-pushed the dev-2.2.0 branch from 6debf66 to 8581b6d Compare February 16, 2024 07:36

jochenklar requested changes May 14, 2024

View reviewed changes

MyPyDavid added 8 commits May 15, 2024 16:16

chore: refactor storeElement signature

2860398

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

chore: add action to methods in Api classes and remove MultiSiteApi

25ed018

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

chore: update element components with signature for storeElements and…

00d2218

… rename hasCurrentSite Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

chore: rename to hasCurrentSite

9d38aa5

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

chore: add element-button class to ReadOnlyIcon for margin-left

3f8a311

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

chore: only return super in CanToggleElementCurrentSite.has_permission

5998074

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

chore: remove unused logger from management/rules

4474043

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

chore: update signature for storeElement in edit components

c56f53b

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

MyPyDavid commented May 15, 2024

View reviewed changes

rdmo/management/assets/js/components/element/Question.js Outdated Show resolved Hide resolved

MyPyDavid requested a review from jochenklar May 15, 2024 15:02

chore: update signature for storeElement to use default parameters

8757d2c

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

MyPyDavid force-pushed the 825-exclude-sites-assignment-from-protection-of-a-catalogue branch from 65ef10e to 8757d2c Compare May 16, 2024 08:09

chore: update signature for storeElement in EditQuestionSet

c8855b9

Signed-off-by: David Wallace <david.wallace@tu-darmstadt.de>

jochenklar requested changes May 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

MyPyDavid commented Dec 8, 2023 •

edited

MyPyDavid commented Dec 8, 2023

jochenklar left a comment

jochenklar Feb 8, 2024

jochenklar May 14, 2024

jochenklar May 14, 2024

jochenklar May 14, 2024

jochenklar May 14, 2024

jochenklar May 17, 2024

jochenklar May 17, 2024

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

Are you sure you want to change the base?

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

Conversation

MyPyDavid commented Dec 8, 2023 • edited

Description

Django back-end

React front-end:

Motivation and Context

How has this been tested?

Screenshots (if appropriate)

Types of Changes

Checklist

MyPyDavid commented Dec 8, 2023

jochenklar left a comment

Choose a reason for hiding this comment

jochenklar Feb 8, 2024

Choose a reason for hiding this comment

jochenklar May 14, 2024

Choose a reason for hiding this comment

jochenklar May 14, 2024

Choose a reason for hiding this comment

jochenklar May 14, 2024

Choose a reason for hiding this comment

jochenklar May 14, 2024

Choose a reason for hiding this comment

jochenklar May 17, 2024

Choose a reason for hiding this comment

jochenklar May 17, 2024

Choose a reason for hiding this comment

MyPyDavid commented Dec 8, 2023 •

edited