feat: add mitxpro-social-auth #376

Anas12091101 · 2024-09-12T14:53:00Z

What are the relevant tickets?

https://github.com/mitodl/hq/issues/5404

Description (What does it do?)

Adds social-auth-mitxpro renamed as ol-social-auth in open-edx-plugins

How can this be tested?

Start edx-platform and mitxpro containers.
Clone this project in the src folder present in your Open edX work directory.
Install pants locally and run the command in the project folder: pants package ::. This will create a tar file ol-social-auth-1.0.0.tar.gz in the dist folder.
Configure Open edX with mitxpro using https://github.com/mitodl/mitxpro/blob/master/docs/configure_open_edx.md if not already and replace social_auth_mitxpro.backends.MITxProOAuth2 with ol_social_auth.backends.MITxProOAuth2 in THIRD_PARTY_AUTH_BACKENDS
Install the tar file in your lms by first creating the lms shell using make lms-shell command in the devstack folder and then pip install /edx/src/open-edx-plugins/dist/ol-social-auth-1.0.0.tar.gz.
Open a private window and create a new account on xpro. Validate that the same user is created in edx-platform.

Additional Context

arslanashraf7

This is just the code review. I will functionality test this in the next iteration.

arslanashraf7 · 2024-09-26T07:51:43Z

.pre-commit-config.yaml

- repo: https://github.com/pre-commit/mirrors-mypy
-  rev: v1.10.1
-  hooks:
-  - id: mypy
-    additional_dependencies:
-    - types-pytz
-    - types-requests
-    - types-pkg_resources
-    - types-python-dateutil


Could you tell me why we have removed it in this PR or any specific reason as to why we are removing it at all?

There were some errors thrown by the pre-commit for mypy therefore, I decided to run the pre-commit without it. I also checked MiTxPRO, MITxOnline and Bootcamp and didn't find the mypy hook.

I'll re-add it in the next commit after resolving the errors

arslanashraf7 · 2024-09-26T07:53:17Z

run_devstack_integration_tests.sh

@@ -21,6 +21,7 @@ cd /open-edx-plugins

 # Installing test dependencies
 pip install pytest-mock==3.14.0
+pip install responses==0.25.3


Isn't it being installed through testing.txt?

Oh, I think I confused it with https://github.com/openedx/edx-platform/blob/master/scripts/user_retirement/requirements/testing.txt#L231

arslanashraf7 · 2024-09-26T07:59:39Z

pyproject.toml

@@ -17,6 +17,7 @@ gitpython = "^3.1.37"
 python-json-logger = "^2.0.2"
 sentry-sdk = "^2.0.0"
 XBlock = "*"
+social-auth-core = "*"


Maybe we should pin this. We don't want to override this package in our open edX instances other than what is being installed by open edx.

arslanashraf7 · 2024-09-26T08:03:51Z

src/ol_social_auth/BUILD

+    ],
+    provides=setup_py(
+        name="ol-social-auth",
+        version="1.0.0",


None of our apps go with 1.x. They all start with a major version. 0.x.x. If we are starting this off we should go with 0.1.0.

I'll change it to 0.1.0 but when we added the openedx-companion-auth plugin, it was also started from 1.0.0 (reference)

I think 0.1.0 is more suitable based on how we use the versioning of our projects.

arslanashraf7 · 2024-09-26T08:06:10Z

src/ol_social_auth/LICENSE

@@ -0,0 +1,29 @@
+BSD 3-Clause License


Is there any LICENSE files that include this line? I think you can copy this from one of the existing LICENSE files from other packages in this repo.

openedx-companion-auth also has this line. Changing both in the next commit

arslanashraf7 · 2024-09-26T08:07:04Z

src/ol_social_auth/BUILD

+    provides=setup_py(
+        name="ol-social-auth",
+        version="1.0.0",
+        description="A package implementing MIT social auth backend",


Suggested change

description="A package implementing MIT social auth backend",

description="An Open edX plugin implementing MIT social auth backend",

arslanashraf7 · 2024-09-26T08:09:22Z

poetry.lock

-# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand.
+# This file is automatically @generated by Poetry 1.6.1 and should not be changed by hand.


Have we downgraded the poetry version? The previous file was generated using 1.8.3 but this time its generated with 1.6.1.

Regenerated with the 1.8.3 version in the next commit

arslanashraf7 · 2024-09-26T08:15:22Z

src/ol_social_auth/README.rst

+Version Compatibility
+---------------------
+
+Compatible with the Nutmeg release of the Open edX and onwards. May work with prior releases as well.


I think this is not correct, Our backend has been working prior to that. Did you copy this from the documentation in the original project?

No, I copied this from openedx-companion-auth Readme. Since both are used together, I thought they would have a similar support. What should I write here? Compatible with all edx releases?

Can you check the history of the original repo and if there is compatibility mentioned there. If not, you can check the history of that repo to see if it is correct. Otherwise, you can skip this heading altogether.

arslanashraf7 · 2024-09-26T08:17:57Z

src/ol_social_auth/README.rst

+* Install the plugin in the lms following the installation steps above.
+* Verify that you are not logged in on edx-platform.
+* Create a new user in your MIT application and verify that a corresponding user is successfully created on the edX platform.


These steps should include that the provider and other configuration be done before testing this.

arslanashraf7 · 2024-09-26T08:19:18Z

src/ol_social_auth/README.rst

+* **MITxOnline with Tutor:** If you're using MITxOnline with Tutor for development purposes, specific configuration steps are outlined in the `documentation <https://github.com/mitodl/mitxonline/blob/main/docs/source/configuration/tutor.rst>`_
+
+
+Working


We should keep things standardized. We are using this heading as How To Use in other plugins.

Anas12091101 · 2024-09-27T07:21:11Z

CI failing due to a recent commit in Tutor in which packaging library was used but the package was not set in the requirements file. A fix is already in progress.

arslanashraf7

The code looks good. But the setup.py file the generate bundle doesn't contain the 'install_requires' section which should have the dependencies for this package.

Anas12091101 · 2024-09-30T09:34:02Z

src/ol_social_auth/BUILD

@@ -0,0 +1,21 @@
+python_sources(
+    name="ol_social_auth_source",
+    dependencies=["//:external_dependencies#social-auth-core"]


pants was not detecting social-auth-core automatically, therefore I added it here.

feat: add mitxpro-social-auth

9435a32

Anas12091101 marked this pull request as draft September 12, 2024 14:55

Anas12091101 added 3 commits September 12, 2024 19:56

fix: github CI

1c9f549

fix: test

f7dea02

fix: precommit errors

4673f01

Anas12091101 marked this pull request as ready for review September 16, 2024 17:55

Anas12091101 added the Needs Review label Sep 16, 2024

Anas12091101 requested a review from arslanashraf7 September 18, 2024 06:15

Anas12091101 added Waiting on Author and removed Needs Review labels Sep 18, 2024

fix: Update and rename README.md to README.rst

9fd31b2

Anas12091101 added Needs Review and removed Waiting on Author labels Sep 18, 2024

arslanashraf7 self-assigned this Sep 23, 2024

arslanashraf7 requested changes Sep 26, 2024

View reviewed changes

arslanashraf7 added Waiting on Author and removed Needs Review labels Sep 26, 2024

fix: issues

d37d22b

Anas12091101 requested a review from arslanashraf7 September 26, 2024 14:15

Anas12091101 added Needs Review and removed Waiting on Author labels Sep 26, 2024

Anas12091101 added 2 commits September 26, 2024 14:18

fix: issues

a8e7537

fix: issues

cb17ce5

arslanashraf7 requested changes Sep 27, 2024

View reviewed changes

arslanashraf7 added Waiting on Author and removed Needs Review labels Sep 27, 2024

fix: fixed install dependencies issue

4af4174

Anas12091101 commented Sep 30, 2024

View reviewed changes

Anas12091101 requested a review from arslanashraf7 September 30, 2024 10:04

Anas12091101 added Needs Review and removed Waiting on Author labels Sep 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add mitxpro-social-auth #376

feat: add mitxpro-social-auth #376

Anas12091101 commented Sep 12, 2024 •

edited

Loading

arslanashraf7 left a comment

arslanashraf7 Sep 26, 2024

Anas12091101 Sep 26, 2024

arslanashraf7 Sep 26, 2024

Anas12091101 Sep 26, 2024

arslanashraf7 Sep 26, 2024

arslanashraf7 Sep 26, 2024

arslanashraf7 Sep 26, 2024

Anas12091101 Sep 26, 2024

arslanashraf7 Sep 26, 2024

arslanashraf7 Sep 26, 2024

Anas12091101 Sep 26, 2024

arslanashraf7 Sep 26, 2024

arslanashraf7 Sep 26, 2024

Anas12091101 Sep 26, 2024 •

edited

Loading

arslanashraf7 Sep 26, 2024

Anas12091101 Sep 26, 2024

arslanashraf7 Sep 26, 2024

arslanashraf7 Sep 26, 2024

arslanashraf7 Sep 26, 2024

Anas12091101 commented Sep 27, 2024

arslanashraf7 left a comment

Anas12091101 Sep 30, 2024

	description="A package implementing MIT social auth backend",
	description="An Open edX plugin implementing MIT social auth backend",

		# This file is automatically @generated by Poetry 1.8.3 and should not be changed by hand.
		# This file is automatically @generated by Poetry 1.6.1 and should not be changed by hand.

		* MITxOnline with Tutor: If you're using MITxOnline with Tutor for development purposes, specific configuration steps are outlined in the `documentation <https://github.com/mitodl/mitxonline/blob/main/docs/source/configuration/tutor.rst>`_


		Working

feat: add mitxpro-social-auth #376

Are you sure you want to change the base?

feat: add mitxpro-social-auth #376

Conversation

Anas12091101 commented Sep 12, 2024 • edited Loading

What are the relevant tickets?

Description (What does it do?)

How can this be tested?

Additional Context

arslanashraf7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anas12091101 Sep 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anas12091101 commented Sep 27, 2024

arslanashraf7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anas12091101 commented Sep 12, 2024 •

edited

Loading

Anas12091101 Sep 26, 2024 •

edited

Loading