edx-enterprise icon indicating copy to clipboard operation
edx-enterprise copied to clipboard

ENT-2663 Plumb detail_fields query param from the api enterpriseCustomerCatalogByUuid gateway.

Open zamanafzal opened this issue 4 years ago • 10 comments

Plumb detail_fields query param from the api enterpriseCustomerCatalogByUuid gateway.

Merge checklist:

  • [ ] Any new requirements are in the right place (do not manually modify the requirements/*.txt files)
    • base.in if needed in production but edx-platform doesn't install it
    • test-master.in if edx-platform pins it, with a matching version
    • make upgrade && make requirements have been run to regenerate requirements
  • [ ] make static has been run to update webpack bundling if any static content was updated
  • [ ] ./manage.py makemigrations has been run
    • Checkout the Database Migration Confluence page for helpful tips on creating migrations.
    • Note: This must be run if you modified any models.
      • It may or may not make a migration depending on exactly what you modified, but it should still be run.
    • This should be run from either a venv with all the lms/edx-enterprise requirements installed or if you checked out edx-enterprise into the src directory used by lms, you can run this command through an lms shell.
      • It would be ./manage.py lms makemigrations in the shell.
  • [ ] Version bumped
  • [ ] Changelog record added
  • [ ] Translations updated (see docs/internationalization.rst but also this isn't blocking for merge atm)

Post merge:

  • [ ] Tag pushed and a new version released
    • Note: Assets will be added automatically. You just need to provide a tag (should match your version number) and title and description.
  • [ ] After versioned build finishes in Travis, verify version has been pushed to PyPI
    • Each step in the release build has a condition flag that checks if the rest of the steps are done and if so will deploy to PyPi. (so basically once your build finishes, after maybe a minute you should see the new version in PyPi automatically (on refresh))
  • [ ] PR created in edx-platform to upgrade dependencies (including edx-enterprise)
    • This must be done after the version is visible in PyPi as make upgrade in edx-platform will look for the latest version in PyPi.
    • Note: the edx-enterprise constraint in edx-platform must also be bumped to the latest version in PyPi.

zamanafzal avatar Nov 20 '20 07:11 zamanafzal

Thanks! Is a corresponding change required in api-compact.yaml? It’s a bit confusing.

mattdrayer avatar Nov 20 '20 12:11 mattdrayer

@mattdrayer yes, you are right. I have made changes in the api-compact.yaml file. Here is the initial PRwhere these changes are made and it shows we need these changes in both files.

zamanafzal avatar Nov 20 '20 13:11 zamanafzal

Excellent, thank you! For testing, I can't recall precisely how to use the stage API gateway -- there is a Postman collection here: https://github.com/edx/edx-postman-config/tree/master/edx -- I think there is a process for updating the Git hash of the API gateway to the desired version in order to test the proper endpoint. I did look in Confluence and found this: https://openedx.atlassian.net/wiki/spaces/~zoldak/pages/99909920/API+Gateway+Testing+and+Deployment and there is also the api-manager README file https://github.com/edx/api-manager

mattdrayer avatar Nov 20 '20 13:11 mattdrayer

@mattdrayer Thank you for sharing the details. I will take a look into it once I got free from my sprint commitments. If you think we need to prioritize it, please create a ticket for its testing.

zamanafzal avatar Nov 23 '20 08:11 zamanafzal

I see this PR is still open -- any reason that it can't be merged?

mattdrayer avatar Jan 27 '21 14:01 mattdrayer

Are we ever going to merge this @zamanafzal?

mattdrayer avatar Feb 22 '21 20:02 mattdrayer

@zamanafzal just saw that this PR is still open/unmerged -- can we get it into the stream at some point soon?

mattdrayer avatar Jul 19 '21 20:07 mattdrayer

ive reopened ENT-2663 and asked the team to review this PR

johnnagro avatar Feb 27 '23 14:02 johnnagro

@mattdrayer can you please confirm that these changes look good to you and only require updating the version number?

zamanafzal avatar Feb 28 '23 12:02 zamanafzal

Supporting the detail_fields query string parameter seems like what we want, so this change looks good, however I can't confirm that the changes to the YAML files + version bump are the only things required for this PR. I've tagged @johnnagro on this PR for verification.

mattdrayer avatar Feb 28 '23 15:02 mattdrayer