openedx / xblock-lti-consumer

GNU Affero General Public License v3.0
28 stars 80 forks source link

fix: Data too long for column 'resource_id' #435

Open shadinaif opened 8 months ago

shadinaif commented 8 months ago

when the course id is a little bit long, the lti xblock id becomes too long for resource_id to handle

To reproduce:

we tried that on the latest https://sandbox.openedx.org

openedx-webhooks commented 8 months ago

Thanks for the pull request, @shadinaif! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

shadinaif commented 8 months ago

For your review @OmarIthawi, please

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.85%. Comparing base (83fa9e0) to head (c5851e6).

:exclamation: Current head c5851e6 differs from pull request most recent head f6fbae4. Consider uploading reports for the commit f6fbae4 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #435 +/- ## ========================================== + Coverage 97.83% 97.85% +0.01% ========================================== Files 77 78 +1 Lines 6710 6709 -1 ========================================== Hits 6565 6565 + Misses 145 144 -1 ``` | [Flag](https://app.codecov.io/gh/openedx/xblock-lti-consumer/pull/435/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openedx) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/openedx/xblock-lti-consumer/pull/435/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openedx) | `97.85% <100.00%> (+0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openedx#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

OmarIthawi commented 8 months ago

Thanks @shadinaif please update the following:

shadinaif commented 8 months ago

please check now @OmarIthawi

mphilbrick211 commented 5 months ago

Hi @OmarIthawi and @shadinaif! Is this pull request still needed?

OmarIthawi commented 5 months ago

Yes @mphilbrick211, we needed it to get merged but it wasn't triaged since Jan 25th.

@shadinaif there's a new changelog update that conflicts with this pull request, kindly rebase and fix it.

shadinaif commented 5 months ago

Rebased. But codecov is failing for no reason ¯\(ツ)

mphilbrick211 commented 2 months ago

Hi @shadinaif and @OmarIthawi! Can this be closed?

OmarIthawi commented 2 months ago

Thanks @shadinaif. @mphilbrick211, this is now ready for merging. I don't have permissions though. Could you please help?