Skip to content

fix: convert this package's own UUID columns in the MariaDB migrations - #2703

Open
blarghmatey wants to merge 2 commits into
openedx:masterfrom
mitodl:fix/mariadb-uuid-conversion-own-tables
Open

blarghmatey wants to merge 2 commits into
openedx:masterfrom
mitodl:fix/mariadb-uuid-conversion-own-tables

Conversation

@blarghmatey

Copy link
Copy Markdown
Contributor

The MariaDB UUID conversions from #2475 in blackboard 0025, canvas 0041 and integrated_channel 0037 alter the enterprise-integrated-channels tables (blackboard_channel_*, canvas_channel_*, channel_integration_*) rather than this package's own. Nothing orders those tables ahead of these migrations, so migrate on a fresh MariaDB database fails at blackboard.0025 with (1146, "Table '...blackboard_channel_blackboardenterprisecustomerconfiguration' doesn't exist"). On existing databases the other package's columns get converted (its own migrations already do that) and this package's columns stay char(32).

This points the three migrations back at this package's tables, and adds blackboard 0026, canvas 0042 and integrated_channel 0038 to convert those tables on databases that already applied the old versions. MODIFY to the type a column already has changes nothing, so the new migrations are also safe after the corrected ones. integrated_channel_contentmetadataitemtransmission.enterprise_customer_catalog_uuid is nullable here, so it is converted as NULL rather than the NOT NULL of the channel_integration copy.

I ran manage.py migrate against MariaDB 11.4 with the test settings pointed at it:

  • master fails at blackboard.0025 with the error above
  • this branch migrates a fresh database, and the four columns end up as uuid
  • with the three new migrations unapplied and the columns set back to char(32) (the state of a database that ran the old versions), migrating converts them to uuid

makemigrations --check reports no changes, and the migration tests pass.

Not covered here: enterprise_customer_uuid on the per-channel audit tables that inherit it from the abstract LearnerDataTransmissionAudit (e.g. blackboard_blackboardlearnerdatatransmissionaudit, canvas_canvaslearnerdatatransmissionaudit) is still char(32). #2475 never converted those either, so I've left them for a follow-up.

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 GitHub Actions, 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)
    • Trigger the 'Upgrade one Python dependency' action against master in edx-platform with new version number to generate version bump PR
    • 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.

https://claude.ai/code/session_01LjapgaiiGBFwNW9PgQ59jv

blackboard 0025, canvas 0041 and integrated_channel 0037 altered the
enterprise-integrated-channels tables (blackboard_channel_*,
canvas_channel_*, channel_integration_*) instead of this package's own.
Nothing orders those tables before these migrations, so on a fresh
MariaDB database migrate fails with "Table ... doesn't exist" at
blackboard 0025. On existing databases the other package's columns were
converted (they already are, by that package's own migrations) and this
package's columns were left as char(32).

The three migrations now name this package's tables, and 0026, 0042 and
0038 convert those tables on databases that applied the old versions.
MODIFY to the type a column already has changes nothing, so the new
migrations are safe after the corrected ones too.

integrated_channel_contentmetadataitemtransmission.enterprise_customer_catalog_uuid
is nullable in this package, so it is converted as NULL rather than the
NOT NULL of the channel_integration copy.

Claude-Session: https://claude.ai/code/session_01LjapgaiiGBFwNW9PgQ59jv
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Sep 30, 2026
@openedx-webhooks

Copy link
Copy Markdown

Thanks for the pull request, @blarghmatey!

This repository is currently maintained by @openedx/2u-enterprise.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@codecov

codecov Bot commented Sep 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.06%. Comparing base (a7d4e6a) to head (bbb6ada).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2703   +/-   ##
=======================================
  Coverage   87.06%   87.06%           
=======================================
  Files         265      265           
  Lines       17280    17280           
  Branches     1709     1709           
=======================================
  Hits        15044    15044           
  Misses       1898     1898           
  Partials      338      338           
Flag Coverage Δ
unittests 87.06% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Status: Needs Triage

Development

Successfully merging this pull request may close these issues.

2 participants