eduNEXT / eox-core

eox-core is a plugin to extend the core functionality in Open edX
GNU Affero General Public License v3.0
15 stars 9 forks source link

fix: remove try except and correct platform paths #217

Closed MaferMazu closed 2 years ago

MaferMazu commented 2 years ago

Description

This PR remove try except from imports in backends and correct platform paths.

Modified Backends:

Testing instructions

  1. Use this strain.yml file to create a mango project
STRAIN_NAME: mango
STRAIN_TUTOR_VERSION: v13.3.1
DEV_PROJECT_NAME: mango_dev
LOCAL_PROJECT_NAME: mango_local
CMS_HOST: studio.mango.edunext.link
LMS_HOST: lms.mango.edunext.link
PLATFORM_NAME: Distro mango
STRAIN_TUTOR_PLUGINS:
  - REPO: tutor-contrib-edunext-distro
    VERSION: v2.3.0
    EGG: tutor-contrib-edunext-distro==v2.3.0
    PACKAGE_NAME: distro
    PROTOCOL: ssh
  - REPO: tutor-forum
    VERSION: 3a5cfac9e68dbac61b8abe4b6e15e023cbb3fee0
    EGG: tutor-forum==3a5cfac9e68dbac61b8abe4b6e15e023cbb3fee0
    PATH: overhangio
    PACKAGE_NAME: forum
STRAIN_EXTRA_COMMANDS:
  - APP: 'tutor'
    COMMAND: 'distro enable-themes'
STRAIN_VOLUME_OVERRIDES:
  edxapp:
    - DESTINATION: ../../src/edxapp/edx-platform
      LOCATION: /openedx/edx-platform
    # Public eox
    - DESTINATION: ../../src/edxapp/eox-core
      LOCATION: /openedx/extra_deps/eox-core
STRAIN_EXPORT_VOLUMES:
  - CONTAINER: lms
    LOCATION: /openedx/edx-platform
    DESTINATION: src/edxapp/edx-platform
  # Public eox
  - CONTAINER: lms
    LOCATION: /openedx/extra_deps/eox-core
    DESTINATION: src/edxapp/eox-core
DISTRO_EOX_TENANT_DPKG: None
DISTRO_EOX_TAGGING_DPKG: None
DISTRO_EOX_HOOKS_DPKG: None
DISTRO_EOX_AUDIT_MODEL_DPKG: None
DISTRO_EOX_MANAGE_DPKG: None
DISTRO_EOX_SUPPORT_DPKG: None
DISTRO_EOX_THEMING_DPKG: None
DISTRO_EOX_CORE_DPKG:
  index: git
  name: eox-core
  private: false
  repo: eox-core
  domain: github.com
  path: eduNEXT
  protocol: https
  variables:
    development:
      EOX_CORE_USERS_BACKEND: eox_core.edxapp_wrapper.backends.users_m_v1
      EOX_CORE_ENROLLMENT_BACKEND: eox_core.edxapp_wrapper.backends.enrollment_l_v1
      EOX_CORE_PRE_ENROLLMENT_BACKEND: eox_core.edxapp_wrapper.backends.pre_enrollment_l_v1
    production:
      EOX_CORE_USERS_BACKEND: eox_core.edxapp_wrapper.backends.users_m_v1
      EOX_CORE_ENROLLMENT_BACKEND: eox_core.edxapp_wrapper.backends.enrollment_l_v1
      EOX_CORE_PRE_ENROLLMENT_BACKEND: eox_core.edxapp_wrapper.backends.pre_enrollment_l_v1
  version: mfmz/correct-backends
  1. Run to create project
stack strain create
source .tvm/bin/activate
stack strain dev configure -k export_vols -k override_vols
stack strain dev init
stack strain dev configure -s export_vols -s override_vols
stack strain dev start
  1. Create a user admin and import a democourse
tutor dev createuser --staff --superuser admin admin@edunext.co -p mangoversion
tutor dev importdemocourse
  1. Create a site. Go to http://lms.mango.edunext.link:8000/admin/site_configuration/siteconfiguration/ and add site configuration, add in site + Domain name: lms.mango.edunext.link:8000. Enable checkbox and save.

  2. Add in https://github.com/eduNEXT/eox-core/blob/master/eox_core/api/task_dispatcher/v1/views.py#L26 the next code lines

from celery import shared_task

@shared_task(bind=True)
def test(self, message):
    """
    Simple task for testing.
    """
    print("-"*60)
    print(message)
    print("-"*60)

    return True
  1. add in https://github.com/eduNEXT/eox-core/blob/master/eox_core/settings/common.py#L50
"eox_core.api.task_dispatcher.v1.views.test"
  1. Follow the steps in eox-core tests. Json file to use in postman

  2. Complete the test cases from 2.6. TESTING APIs WITH A USER WITHOUT STAFF PRIVILEGES. test cases file

Checklist for Merge

MaferMazu commented 2 years ago

@JuanDavidBuitrago , did you test this change with all test cases?

I ask because If you tested all, I think the PR is OK, but usually, when you delete try-except, the test fails if you don't have backends for tests. I tried to run the python-test (make python-test) in a venv with python38, and the output was: Screenshot from 2022-10-03 08-58-55 I thought it was weird because in github you have a check, but reviewing that test, I found: Screenshot from 2022-10-03 09-09-34 We are skipping tests.

I don't think this is the right PR to correct that, but we need to look at the nutmeg migration because I believe we are not testing well.

JuanDavidBuitrago commented 2 years ago

@JuanDavidBuitrago , did you test this change with all test cases?

I ask because If you tested all, I think the PR is OK, but usually, when you delete try-except, the test fails if you don't have backends for tests. I tried to run the python-test (make python-test) in a venv with python38, and the output was: Screenshot from 2022-10-03 08-58-55 I thought it was weird because in github you have a check, but reviewing that test, I found: Screenshot from 2022-10-03 09-09-34 We are skipping tests.

I don't think this is the right PR to correct that, but we need to look at the nutmeg migration because I believe we are not testing well.

Yes, I did test with all test cases. It's working well.

I think to adding test, we have to work on this in a specific sprint, there are so much files that need changes but not affect the plugin functionality. All test cases are working without any problem and I think that is enough for now while we plan to improve the test code.