Skip to content

Commit

Permalink
✨(back) allow all users created via shibboleth to have a playlist
Browse files Browse the repository at this point in the history
We want to allow, when we want, users to have an associated playlist
when they create their account via shibboleth. For this a new waffle
switch is used.
  • Loading branch information
lunika committed Oct 22, 2024
1 parent 2c4d0be commit ba46e26
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 6 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Added

- Allow all users created via shibboleth to have their own playlist

## [5.1.0] - 2024-10-09

### Added
Expand Down
17 changes: 11 additions & 6 deletions src/backend/marsha/account/social_pipeline/playlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
from django.conf import settings
from django.db.transaction import atomic

from waffle import switch_is_active

from marsha.account.models import IdpOrganizationAssociation
from marsha.core.defaults import ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES
from marsha.core.models import ADMINISTRATOR, INSTRUCTOR, Playlist, PlaylistAccess


Expand Down Expand Up @@ -90,13 +93,15 @@ def create_playlist_from_saml( # pylint:disable=too-many-arguments
# Only create a new playlist for new user association.
return

is_instructor = details.get("roles", None) and any(
role in details["roles"] for role in settings.SOCIAL_AUTH_SAML_FER_TEACHER_ROLES
)
if not switch_is_active(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES):
is_instructor = details.get("roles", None) and any(
role in details["roles"]
for role in settings.SOCIAL_AUTH_SAML_FER_TEACHER_ROLES
)

# If the user has no instructor role we don't create a new playlist.
if not is_instructor:
return
# If the user has no instructor role we don't create a new playlist.
if not is_instructor:
return

idp = backend.get_idp(response["idp_name"])

Expand Down
53 changes: 53 additions & 0 deletions src/backend/marsha/account/tests/test_social_pipeline_playlist.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
from django.test import TestCase

from social_django.utils import load_strategy
from waffle.testutils import override_switch

from marsha.account.factories import IdpOrganizationAssociationFactory
from marsha.account.social_pipeline.playlist import create_playlist_from_saml
from marsha.account.tests.utils import MockedFERSAMLAuth, override_saml_fer_settings
from marsha.core.defaults import ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES
from marsha.core.factories import PlaylistAccessFactory, UserFactory
from marsha.core.models import ADMINISTRATOR, INSTRUCTOR, Playlist, PlaylistAccess

Expand Down Expand Up @@ -58,6 +60,7 @@ def setUp(self) -> None:
self.assertEqual(Playlist.objects.count(), 0)
self.assertEqual(PlaylistAccess.objects.count(), 0)

@override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False)
def test_create_playlist_from_saml_student_or_unknown_new(self):
"""No playlist should be created when the user is a student."""

Expand All @@ -75,6 +78,7 @@ def test_create_playlist_from_saml_student_or_unknown_new(self):
self.assertEqual(Playlist.objects.count(), 0)
self.assertEqual(PlaylistAccess.objects.count(), 0)

@override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False)
def test_create_playlist_no_new_association(self):
"""When new_association is False, no playlist should be created."""
saml_details = {"roles": ["teacher"], **self.default_saml_details}
Expand All @@ -91,6 +95,7 @@ def test_create_playlist_no_new_association(self):
self.assertEqual(Playlist.objects.count(), 0)
self.assertEqual(PlaylistAccess.objects.count(), 0)

@override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False)
def test_create_playlist_new_association_no_existing_idporganization(self):
"""With a new association but no existing IdpOrganizationAssociation
an exception should be raised"""
Expand All @@ -107,6 +112,7 @@ def test_create_playlist_new_association_no_existing_idporganization(self):
new_association=True,
)

@override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False)
def test_create_playlist_new_association_existing_idporganization(self):
"""A new playlist should be created when new_association is True
and an idpOrganizationAssociation exists."""
Expand Down Expand Up @@ -136,6 +142,7 @@ def test_create_playlist_new_association_existing_idporganization(self):
self.assertEqual(playlist_access.user, self.user)
self.assertEqual(playlist_access.role, ADMINISTRATOR)

@override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False)
def test_create_playlist_new_association_existing_idp_organization_user_full_name(
self,
):
Expand Down Expand Up @@ -182,6 +189,7 @@ def test_create_playlist_new_association_existing_idp_organization_user_full_nam
self.assertEqual(playlist_access.user, user)
self.assertEqual(playlist_access.role, ADMINISTRATOR)

@override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=False)
def test_create_playlist_already_existing_playlist_access(self):
"""No playlist should be created if an existing playlist access for the user and the
organization is found."""
Expand Down Expand Up @@ -211,3 +219,48 @@ def test_create_playlist_already_existing_playlist_access(self):

self.assertEqual(Playlist.objects.count(), 1)
self.assertEqual(PlaylistAccess.objects.count(), 1)

@override_switch(ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES, active=True)
def test_create_playlist_student_active_flag(
self,
):
"""A new playlist hsould be created for a student when the flag is active."""

saml_details = {
"roles": ["student"],
"first_name": "jon",
"last_name": "snow",
"username": "[email protected]", # unused
"email": "[email protected]", # unused
}

user = UserFactory(
first_name=saml_details["first_name"],
last_name=saml_details["last_name"],
username=saml_details["username"],
email=saml_details["email"],
)

idp_organization_association = IdpOrganizationAssociationFactory(
idp_identifier="http://marcha.local/idp/"
)

create_playlist_from_saml(
None, # strategy is unused
saml_details,
self.backend,
user=user,
response=self.saml_response,
new_association=True,
)

playlist = Playlist.objects.last()
playlist_access = PlaylistAccess.objects.last()

self.assertEqual(
playlist.organization, idp_organization_association.organization
)
self.assertEqual(playlist.title, user.get_full_name())
self.assertEqual(playlist_access.playlist, playlist)
self.assertEqual(playlist_access.user, user)
self.assertEqual(playlist_access.role, ADMINISTRATOR)
1 change: 1 addition & 0 deletions src/backend/marsha/core/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
RENATER_FER_SAML = "renater_fer_saml"
VOD_CONVERT = "vod_convert"
TRANSCRIPTION = "transcription"
ALLOW_PLAYLIST_CREATION_FOR_ALL_ROLES = "allow_playlist_creation_for_all_roles"

# LTI view states expected by the frontend LTI application
APP_DATA_STATE_ERROR = "error"
Expand Down

0 comments on commit ba46e26

Please sign in to comment.