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

Jlc/add cms support for admin and course creator #2

Merged
merged 5 commits into from
Aug 22, 2022

Conversation

johanseto
Copy link
Collaborator

@johanseto johanseto commented Aug 19, 2022

Description

This is related to the requirement of Nelp that is creating users but need to give course creator permissions.
The problem is that until the user login, the user will not appear in the course-creator table.
https://github.com/eduNEXT/edx-platform/blob/master/cms/djangoapps/contentstore/views/course.py#L1889-L1893
So this PR adds the option to add course creator directly from the admin model matching with the username of the user model.
Development
This is a remake of eduNEXT/edx-platform#269 using the eox-nelpplugin.
Configuration
Remember to configure you have to update your cms/env/devstack.py or your custom settings.

FEATURES['ENABLE_CREATOR_GROUP'] = True.
Note: Is already in stage environment to check.
https://studio.stage.nelp.gov.sa/admin/course_creators/coursecreator/add/

Jira story

https://edunext.atlassian.net/browse/NELP-195?atlOrigin=eyJpIjoiNmFmOTc4YTk4NDczNGZiZGEwZjA2YzY5MjdlYzljNjQiLCJwIjoiaiJ9

Before

2022-08-18_18-03

After

2022-08-18_17-19
2022-08-18_18-10

@johanseto johanseto force-pushed the jlc/add-cms-support-for-admin-and-course-creator branch from fb9517d to 3600012 Compare August 19, 2022 20:15
This allow modify the course creator model admin using the eox-nelp plugin.
This change  add the possibilty to add or delete course creator from admin.
Due course_creators is an studio app, when lms use `eox_nelp` it bring an error.
This avoid the following error ```RuntimeError: Model class cms.djangoapps.course_creators.models.CourseCreator doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS```.
So you have to add this one in the `INSTALLED_APPS`.
@johanseto johanseto force-pushed the jlc/add-cms-support-for-admin-and-course-creator branch from f98aeb6 to edd5ed2 Compare August 19, 2022 21:01
@@ -0,0 +1,23 @@
"""Backend for course_creators module.
Copy link
Collaborator

Choose a reason for hiding this comment

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

the convention is file_name_{first letter of the version}_v1.py so for this case this should be course_creators_k_v1.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actually, I put lilac cause is like a reference. Most of the eox-core backends don't have the koa version, and instead, use the lilac. That is the reason I set Lilac.
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

They didn't test eox-core with koa so I think that is the reason. Anyway you can not ensure that this backend will work in lilac therefore you should set the name with the version that you use

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

OK, you have a good point. Let me change it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed in 71e8c18

Comment on lines 30 to 33
def has_add_permission(self, request):
return True
def has_delete_permission(self, request, obj=None):
return True
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def has_add_permission(self, request):
return True
def has_delete_permission(self, request, obj=None):
return True
def has_add_permission(self, request):
return True
def has_delete_permission(self, request, obj=None):
return True

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved in a125a75

classes:
nelpCourseCreatorAdmin: EoxNelp CourseCreators admin class.
"""
from __future__ import absolute_import
Copy link
Collaborator

Choose a reason for hiding this comment

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

generally we use this in applications that has support for python 2 and 3, this is just for python 3 so do you need this for something else ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, I think no cause we are using python3, I will remove it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved in a125a75

This contains all the required dependencies from course_creators.
Attributes:
backend:Imported ccx module by using the plugin settings.
CourseCreator: Wrapper courseCreator model.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
CourseCreator: Wrapper courseCreator model.
CourseCreator: Wrapper CourseCreator model.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved in a125a75

"""Wrapper course_creator module file.
This contains all the required dependencies from course_creators.
Attributes:
backend:Imported ccx module by using the plugin settings.
Copy link
Collaborator

Choose a reason for hiding this comment

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

ccx?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved in a125a75

eox_nelp/apps.py Outdated
class EoxNelpCMSConfig(AppConfig):
"""App configuration"""
name = 'eox_nelp'
verbose_name = "eduNEXT Openedx Extensions"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@andrey-canon what is the problem? THe name???

Copy link
Collaborator

Choose a reason for hiding this comment

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

this could be Nelp Openedx Extension since "eduNEXT Openedx Extensions" is eox-core

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah ok heheh let me I change it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved in a125a75



class NelpCourseCreatorAdmin(CourseCreatorAdmin):
"""EoxSupport CertificateTemplate admin class.
Copy link
Collaborator

Choose a reason for hiding this comment

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

EoxSupport? CertificateTemplate ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved in a125a75

@@ -0,0 +1,36 @@
"""courseCreators admin file.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"""courseCreators admin file.
"""CourseCreators admin file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved in a125a75

"""courseCreators admin file.
Contains all the nelped admin models for course_creators.
classes:
nelpCourseCreatorAdmin: EoxNelp CourseCreators admin class.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
nelpCourseCreatorAdmin: EoxNelp CourseCreators admin class.
NelpCourseCreatorAdmin: EoxNelp CourseCreators admin class.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved in a125a75

Due this backend was backend was tested in koa it could add uncertainty affirm that would work in lilac too.
@johanseto johanseto merged commit b623854 into master Aug 22, 2022
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