Skip to content

Simplify in-context discussion toggles [BD-38] - #339

Merged
asadazam93 merged 3 commits into
openedx:masterfrom
open-craft:tecoholic/simplify-toggles
Sep 7, 2022
Merged

Simplify in-context discussion toggles [BD-38]#339
asadazam93 merged 3 commits into
openedx:masterfrom
open-craft:tecoholic/simplify-toggles

Conversation

@tecoholic

Copy link
Copy Markdown
Contributor

Description

Implements the following changes:

  1. Following toggles will be removed:
    • Allow visibility configuration for each course unit
    • In-context discussion
  2. Following toggles will be renamed:
    • "Graded unit pages" will be renamed to "Enable discussions on units in graded subsection"

Testing instructions

  1. The test setup uses the demo course setup in devstack and the app running in localhost:2001
  2. Change the Discussion provider value to openedx in http://localhost:18000/admin/discussions/discussionsconfiguration/ for the demo course
  3. Go to http://localhost:2001/course/course-v1:edX+DemoX+Demo_Course/pages-and-resources/discussion/configure/openedx
  4. Check the changes are implemented as expected.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 25, 2022
@openedx-webhooks

openedx-webhooks commented Aug 25, 2022

Copy link
Copy Markdown

Thanks for the pull request, @tecoholic!

When this pull request is ready, tag your edX technical lead.

@codecov

codecov Bot commented Aug 26, 2022

Copy link
Copy Markdown

Codecov Report

Base: 73.89% // Head: 73.95% // Increases project coverage by +0.06% 🎉

Coverage data is based on head (085d6ca) compared to base (8dfac20).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #339      +/-   ##
==========================================
+ Coverage   73.89%   73.95%   +0.06%     
==========================================
  Files         105      105              
  Lines        1965     1962       -3     
  Branches      475      472       -3     
==========================================
- Hits         1452     1451       -1     
+ Misses        485      484       -1     
+ Partials       28       27       -1     
Impacted Files Coverage Δ
...app-config-form/apps/openedx/OpenedXConfigForm.jsx 83.78% <ø> (ø)
...fig-form/apps/shared/InContextDiscussionFields.jsx 100.00% <ø> (+33.33%) ⬆️
...-resources/discussions/app-config-form/messages.js 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@natabene

Copy link
Copy Markdown

@tecoholic Thank you for the contribution, is this ready for our review?

@tecoholic

Copy link
Copy Markdown
Contributor Author

@natabene Yes. This is ready for review.
cc: @xitij2000

@xitij2000 xitij2000 changed the title Simplify in-context discussion toggles Simplify in-context discussion toggles [BD-38] Sep 1, 2022
@openedx-webhooks openedx-webhooks added blended PR is managed through 2U's blended developmnt program and removed open-source-contribution PR author is not from Axim or 2U labels Sep 1, 2022

@xitij2000 xitij2000 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 Looks good!

  • I tested this: tested on devstack
  • I read through the code

Comment thread src/pages-and-resources/discussions/app-config-form/messages.js Outdated
@tecoholic
tecoholic force-pushed the tecoholic/simplify-toggles branch from c4b9e8f to 085d6ca Compare September 5, 2022 06:41
@asadazam93

Copy link
Copy Markdown
Contributor

@xitij2000 Does this have any related Jira ticket?

@natabene

natabene commented Sep 6, 2022

Copy link
Copy Markdown

@asadazam93 No, you will need to create one if you need it.

@asadazam93
asadazam93 merged commit 38f9f68 into openedx:master Sep 7, 2022
@openedx-webhooks

Copy link
Copy Markdown

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

@xitij2000
xitij2000 deleted the tecoholic/simplify-toggles branch September 13, 2022 12:14
rpenido pushed a commit to open-craft/frontend-app-authoring that referenced this pull request Jan 2, 2024
bradenmacdonald pushed a commit to open-craft/frontend-app-authoring that referenced this pull request Aug 9, 2024
…e-react

Revert "chore: update react to 17, etc. TNL-10715"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blended PR is managed through 2U's blended developmnt program

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants