Skip to content

refactor: modified the imports to use openedx-events library #29930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
asadazam93 merged 3 commits into from
Mar 7, 2022
Merged

refactor: modified the imports to use openedx-events library #29930

asadazam93 merged 3 commits into from
Mar 7, 2022

Conversation

@JuanDavidBuitrago
Copy link
Contributor

@JuanDavidBuitrago JuanDavidBuitrago commented Feb 15, 2022 •
edited
Loading

Description

This PR modified the imports to use openedx-events library, it's due to the extraction of the COURSE_DISCUSSIONS_CHANGED event definition and his attr data was move from edx-platform to openedx-events.

Supporting information

Discussion event extraction PR

Related PR

Testing instructions

  1. In studio, modified the content of a course in a unit and publish changes.
  2. You should see new DiscussionTopicLink objects.
  3. In studio logs you can see log.info for "Updating existing discussion topic links for" or "Creating new discussion topic links for".

You can use the instructions from PR #29082 too.

Deadline

"None"

Other information

@openedx-webhooks
Copy link

openedx-webhooks commented Feb 15, 2022 •
edited
Loading

Thanks for the pull request, @JuanDavidBuitrago! I've created OSPR-6462 to keep track of it in JIRA, where we prioritize reviews. 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:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

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.

@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Feb 15, 2022
@natabene
Copy link
Contributor

natabene commented Feb 16, 2022

@JuanDavidBuitrago Thank you for your contribution. Please let me know once this is ready for our review.

@JuanDavidBuitrago JuanDavidBuitrago force-pushed the JDB/imports_of_discussion_event branch from 12d360c to 35bb7c5 Compare February 25, 2022 19:14
@JuanDavidBuitrago JuanDavidBuitrago marked this pull request as ready for review February 25, 2022 19:40
@openedx-webhooks openedx-webhooks added needs triage and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Feb 25, 2022
@JuanDavidBuitrago
Copy link
Contributor Author

JuanDavidBuitrago commented Feb 25, 2022

Hi everyone!!!

This PR is ready for review.
@natabene, @xitij2000, @felipemontoya and @mariajgrimaldi, Could you please check out the PR?

I will be attentive to your comments.
Thanks

@felipemontoya
Copy link
Member

felipemontoya commented Feb 25, 2022

@JuanDavidBuitrago this PR would require you to bump openedx-events to 0.8.0 right?

@JuanDavidBuitrago JuanDavidBuitrago force-pushed the JDB/imports_of_discussion_event branch from 57f4cb8 to c79d209 Compare March 1, 2022 13:44
@JuanDavidBuitrago
Copy link
Contributor Author

JuanDavidBuitrago commented Mar 1, 2022

@JuanDavidBuitrago this PR would require you to bump openedx-events to 0.8.0 right?

Yes @felipemontoya, it's done!

@JuanDavidBuitrago JuanDavidBuitrago changed the title feat: modified the imports to use openedx-events library refactor: modified the imports to use openedx-events library Mar 1, 2022
@JuanDavidBuitrago JuanDavidBuitrago force-pushed the JDB/imports_of_discussion_event branch 3 times, most recently from b3e01ea to df704b9 Compare March 1, 2022 18:08
@mariajgrimaldi mariajgrimaldi mentioned this pull request Mar 2, 2022
Merged
9 tasks
@mariajgrimaldi
Copy link
Member

mariajgrimaldi commented Mar 2, 2022

I fixed the issue that caused tests to fail: PR

@mariajgrimaldi mariajgrimaldi force-pushed the JDB/imports_of_discussion_event branch from df704b9 to 5dcaa62 Compare March 3, 2022 15:46
@mariajgrimaldi
Copy link
Member

mariajgrimaldi commented Mar 4, 2022 •
edited
Loading

Hi there @felipemontoya, I fixed the tests over here :)

Hi, you @xitij2000! Can you give us a hand? 😄

felipemontoya
felipemontoya approved these changes Mar 4, 2022
View reviewed changes
Copy link
Member

@felipemontoya felipemontoya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to merge now. Thanks @JuanDavidBuitrago and @mariajgrimaldi.

@xitij2000
Copy link
Contributor

xitij2000 commented Mar 7, 2022

Hi there @felipemontoya, I fixed the tests over here :)

Hi, you @xitij2000! Can you give us a hand? smile

I just checked out this PR and tested the code, it seems to we working well :-)

mariajgrimaldi
mariajgrimaldi approved these changes Mar 7, 2022
View reviewed changes
Copy link
Member

@mariajgrimaldi mariajgrimaldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also tested this, and it seemed to work just fine. I'll contact the owning team to merge this :)

@asadazam93 asadazam93 merged commit 860dfde into openedx:master Mar 7, 2022
@openedx-webhooks
Copy link

openedx-webhooks commented Mar 7, 2022

@JuanDavidBuitrago 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Mar 7, 2022

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@JuanDavidBuitrago JuanDavidBuitrago deleted the JDB/imports_of_discussion_event branch March 7, 2022 15:59
@edx-pipeline-bot
Copy link
Contributor

edx-pipeline-bot commented Mar 7, 2022

EdX Release Notice: This PR has been deployed to the production environment.

@natabene
Copy link
Contributor

natabene commented Mar 8, 2022

@JuanDavidBuitrago Thank you for your contribution.

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

Reviewers

@felipemontoya felipemontoya felipemontoya approved these changes

@mariajgrimaldi mariajgrimaldi mariajgrimaldi approved these changes

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@JuanDavidBuitrago @openedx-webhooks @natabene @felipemontoya @mariajgrimaldi @xitij2000 @edx-pipeline-bot @asadazam93