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.

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

feat: remove staff access in courses for catalog views #37

Merged
merged 5 commits into from
Sep 16, 2024

Conversation

johanseto
Copy link
Collaborator

@johanseto johanseto commented Sep 5, 2024

Description

This is controversial, but simple. To do this in custom eox-nelp with a monkey patch is a bigger task and also with more work and risk. Is different due the change is in a nested function. Anyway the alternative is to monkeyPatch the _dispatch method and handle there. But its risky and also that method manage other permission with different ways.

Testing instructions

HIde a course in studio and check that for anonymous_user doesn't load but for staff general yes.
before_1

Before

staff
before_1
anonymous_user
before_2

After

staff
after_1
anonymous_user
after_2

Other information

Jira Story:

@johanseto johanseto force-pushed the jlc/remove-staff-courses-home-page branch 2 times, most recently from c9d46af to 1fbf18c Compare September 5, 2024 22:33
@johanseto johanseto changed the title feat: remove staff access for catalog views feat: remove staff access in courses for catalog views Sep 5, 2024
@@ -394,7 +394,6 @@ def can_see_in_catalog():
"""
return (
_has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
or _has_staff_access_to_descriptor(user, courselike, courselike.id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead of removing this, could you add a feature flag inside the _has_staff_access_to_descriptor method that allows to keep standard and custom behavior ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I could add a feature flag, but I only want to change can_see_in_catalog. I don't want to affect many other places where the staff could have permission.
Do you agree to add the feature flag only here?

Copy link
Collaborator

@andrey-canon andrey-canon left a comment

Choose a reason for hiding this comment

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

Please don't forget to update the migration file

This is controversial, but simple. To do this in a monkey patch is a bigger task and also with
more work. Is different due the change is in a nested function. Anyway the alternative is to
monkeyPatch the _dispatch method and handle there. But its risky and also that method manage other permission with different ways.
@johanseto johanseto force-pushed the jlc/remove-staff-courses-home-page branch from 7faf860 to a29df93 Compare September 16, 2024 15:13
@johanseto johanseto merged commit 3381de6 into open-release/palm.nelp Sep 16, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants