feat: [FC-0044] group configurations MFE page #929

ruzniaievdm · 2024-03-31T19:14:20Z

Settings

EDX_PLATFORM_REPOSITORY: https://github.com/raccoongang/edx-platform.git
EDX_PLATFORM_VERSION: ts-develop
TUTOR_GROVE_WAFFLE_FLAGS:
  - name: contentstore.new_studio_mfe.use_new_group_configurations_page
    everyone: true

Issue (figma url contains)

openedx/platform-roadmap#318

Learn about group configurations:

How to activate content experiments configure: https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_features/content_experiments/content_experiments_configure.html#id11
How to add content group: https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_features/cohorts/cohorted_courseware.html#content-groups
Creating Course Content for Specific Enrollment Tracks https://edx.readthedocs.io/projects/open-edx-building-and-running-a-course/en/latest/course_features/diff_content/enroll_track_courseware.html

Testing instructions

Run master devstack.
Start platform make dev.up.lms+cms + frontend-app-course-authoring and make checkout on this branch.
Enable the new Group Configurations page by adding a waffle flag contentstore.new_studio_mfe.use_new_group_configurations_page in the CMS admin panel.
Go to the Group Configurations page.

Demo

Content Group

Screen.Recording.Content.Group.mov

Enrollment Track group

Screen.Recording.Enrollment.Track.Group.mov

Experiment configurations

Screen.Recording.Experiment.Configurations.mov

openedx-webhooks · 2024-03-31T19:14:25Z

Thanks for the pull request, @ruzniaievdm! 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.

codecov · 2024-04-01T10:03:11Z

Codecov Report

Attention: Patch coverage is 91.97652% with 41 lines in your changes are missing coverage. Please review.

Project coverage is 92.01%. Comparing base (7f3164b) to head (bd8e686).
Report is 13 commits behind head on master.

❗ Current head bd8e686 differs from pull request most recent head 1b7a9c4. Consider uploading reports for the commit 1b7a9c4 to get more accurate results

Files	Patch %	Lines
src/group-configurations/data/thunk.js	84.21%	12 Missing ⚠️
...ations/content-groups-section/ContentGroupCard.jsx	79.41%	5 Missing and 2 partials ⚠️
...periment-configurations-section/ExperimentCard.jsx	84.61%	5 Missing and 1 partial ⚠️
...up-configurations/content-groups-section/index.jsx	79.16%	5 Missing ⚠️
...periment-configurations-section/ExperimentForm.jsx	92.00%	2 Missing ⚠️
...nt-configurations-section/ExperimentFormGroups.jsx	88.88%	2 Missing ⚠️
...ations/experiment-configurations-section/index.jsx	89.47%	2 Missing ⚠️
src/hooks.js	60.00%	2 Missing ⚠️
src/group-configurations/data/api.js	96.55%	1 Missing ⚠️
src/group-configurations/data/slice.js	97.14%	1 Missing ⚠️
... and 1 more

Additional details and impacted files

@@           Coverage Diff            @@
##           master     #929    +/-   ##
========================================
  Coverage   92.00%   92.01%            
========================================
  Files         612      645    +33     
  Lines       10745    11255   +510     
  Branches     2304     2431   +127     
========================================
+ Hits         9886    10356   +470     
- Misses        830      867    +37     
- Partials       29       32     +3

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

* feat: [AXIMST-63] Index group configurations page * fix: resolve discussions * fix: resolve second round discussions

* feat: [AXIMST-75, AXIMST-69, AXIMST-81] Content group actions * fix: resolve conversations

* feat: [AXIMST-87] group-configuration page sidebar * refactor: [AXIMST-87] add changes after review * refactor: [AXIMST-87] add changes after review * refactor: [AXIMST-87] add changes ater review --------- Co-authored-by: Kyrylo Hudym-Levkovych <[email protected]>

…nges

* feat: [AXIMST-93, 99, 105] Group configuration - Experiment Groups * fix: [AXIMST-518, 537] Group configuration - resolve bugs * fix: review discussions * fix: revert classname case

fix: [AXIMST-714] icon is aligned with text (#210)

open-craft-grove · 2024-04-07T13:04:59Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

KristinAoki · 2024-04-08T14:12:44Z

src/group-configurations/content-groups-section/ContentGroupForm.jsx

+                {isEditMode && (
+                  <OverlayTrigger
+                    overlay={(
+                      <Tooltip
+                        id={`delete-restriction-tooltip-${values.newGroupName}`}
+                      >
+                        {formatMessage(
+                          isUsedInLocation
+                            ? messages.deleteRestriction
+                            : messages.deleteButton,
+                        )}
+                      </Tooltip>
+                    )}
+                  >
+                    <Button
+                      disabled={isUsedInLocation}
+                      variant="outline-primary"
+                      onClick={onDeleteClick}
+                    >
+                      {formatMessage(messages.deleteButton)}
+                    </Button>
+                  </OverlayTrigger>
+                )}


Remove, the delete option should only be shown in the non-edit mode

No problem, will be removed
Do we need to apply the same logic for the experiment configurations container?

KristinAoki · 2024-04-08T14:36:54Z

src/group-configurations/messages.js

+  containsGroups: {
+    id: 'course-authoring.group-configurations.container.contains-groups',
+    defaultMessage: 'Contains {len} groups',
+    description: 'Message indicating the number of groups contained within a container.',
+  },
+  containsGroup: {
+    id: 'course-authoring.group-configurations.container.contains-group',
+    defaultMessage: 'Contains 1 group',
+    description: 'Message indicating that there is only one group contained within a container.',
+  },


These can be combined into one message

Suggested change

containsGroups: {

id: 'course-authoring.group-configurations.container.contains-groups',

defaultMessage: 'Contains {len} groups',

description: 'Message indicating the number of groups contained within a container.',

},

containsGroup: {

id: 'course-authoring.group-configurations.container.contains-group',

defaultMessage: 'Contains 1 group',

description: 'Message indicating that there is only one group contained within a container.',

},

containsGroups: {

id: 'course-authoring.group-configurations.container.contains-groups',

defaultMessage: 'Contains {len, plural, one {group} other {groups}}',

description: 'Message indicating the number of groups contained within a container.',

},

It seems we have only two options. There are groups with contains groups and contains group without a counter. Am I understanding this correctly?

This update utilizes the plural function available with intl() based on the value of len. If len equals one, it will use "group". If 'len' equals any other number, it will use "groups". This is a reduces the amount of messages required.

containsGroups: { id: 'course-authoring.group-configurations.container.contains-groups', defaultMessage: 'Contains {len, plural, one {group} other {groups}}', description: 'Message indicating the number of groups contained within a container.', },

The defaultMessage should be

containsGroups: { id: 'course-authoring.group-configurations.container.contains-groups', defaultMessage: 'Contains {len, plural, one {1 group} other {{len} groups}}', description: 'Message indicating the number of groups contained within a container.', },

KristinAoki · 2024-04-08T14:37:38Z

src/group-configurations/messages.js

+  usedInLocations: {
+    id: 'course-authoring.group-configurations.container.used-in-locations',
+    defaultMessage: 'Used in {len} locations',
+    description: 'Message indicating the number of locations where the group configurations are used.',
+  },
+  usedInLocation: {
+    id: 'course-authoring.group-configurations.container.used-in-location',
+    defaultMessage: 'Used in 1 location',
+    description: 'Message indicating that the group configurations are used in only one location.',
+  },


Suggested change

usedInLocations: {

id: 'course-authoring.group-configurations.container.used-in-locations',

defaultMessage: 'Used in {len} locations',

description: 'Message indicating the number of locations where the group configurations are used.',

},

usedInLocation: {

id: 'course-authoring.group-configurations.container.used-in-location',

defaultMessage: 'Used in 1 location',

description: 'Message indicating that the group configurations are used in only one location.',

},

usedInLocations: {

id: 'course-authoring.group-configurations.container.used-in-locations',

defaultMessage: 'Used in {len, plural, one {location} other {locations}',

description: 'Message indicating the number of locations where the group configurations are used.',

},

usedInLocations: { id: 'course-authoring.group-configurations.container.used-in-locations', defaultMessage: 'Used in {len, plural, one {location} other {locations}', description: 'Message indicating the number of locations where the group configurations are used.' },

The defaultMessage should be

usedInLocations: { id: 'course-authoring.group-configurations.container.used-in-locations', defaultMessage: 'Used in {len, plural, one {1 location} other {{len} locations}', description: 'Message indicating the number of locations where the group configurations are used.' },

KristinAoki · 2024-04-08T14:56:44Z

src/group-configurations/empty-placeholder/EmptyPlaceholder.test.jsx

Will you add a test that checks the experimental title and button render as expected?

Do you mean update according to the changes above? Or do you want to cover this functionality because the tests are already added here ExperimentCard.test.jsx

I missed that. You can ignore this comment.

open-craft-grove · 2024-04-15T09:21:59Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

open-craft-grove · 2024-04-15T09:55:20Z

Sandbox deployment successful 🚀
🎓 LMS
📝 Studio
ℹ️ Grove Config, Tutor Config, Tutor Requirements

openedx-webhooks · 2024-04-23T15:53:52Z

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

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 31, 2024

ruzniaievdm force-pushed the ruzniaievdm/feat/group-configurations-feature branch from 5afea18 to 9d30839 Compare April 1, 2024 09:53

ruzniaievdm force-pushed the ruzniaievdm/feat/group-configurations-feature branch 3 times, most recently from 2652b60 to 5da4665 Compare April 3, 2024 07:38

ruzniaievdm marked this pull request as ready for review April 4, 2024 13:15

ruzniaievdm requested a review from a team as a code owner April 4, 2024 13:15

ruzniaievdm and others added 6 commits April 5, 2024 12:11

feat: group configurations - index page

583ff77

* feat: [AXIMST-63] Index group configurations page * fix: resolve discussions * fix: resolve second round discussions

feat: group configurations - content group actions

dd19efb

* feat: [AXIMST-75, AXIMST-69, AXIMST-81] Content group actions * fix: resolve conversations

fix: group configurations - the page reloads after the user saves cha…

0a07a10

…nges

feat: group configurations - experiment groups

62887a5

* feat: [AXIMST-93, 99, 105] Group configuration - Experiment Groups * fix: [AXIMST-518, 537] Group configuration - resolve bugs * fix: review discussions * fix: revert classname case

fix: group configurations - resolve discussions

111917b

fix: [AXIMST-714] icon is aligned with text (#210)

ruzniaievdm force-pushed the ruzniaievdm/feat/group-configurations-feature branch from 1682aac to 111917b Compare April 5, 2024 09:11

monteri added 3 commits April 5, 2024 12:19

fix: add hook tests

b3ee1bd

fix: add thunk tests

86598b6

fix: add slice tests

ae15f69

ruzniaievdm force-pushed the ruzniaievdm/feat/group-configurations-feature branch from 66fd541 to ae15f69 Compare April 5, 2024 16:29

chore: group configurations - messages

bd8e686

PKulkoRaccoonGang added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Apr 7, 2024

KristinAoki requested changes Apr 8, 2024

View reviewed changes

fix: group configurations - remove delete in edit mode

1b7a9c4

ruzniaievdm force-pushed the ruzniaievdm/feat/group-configurations-feature branch from 0b72443 to 1b7a9c4 Compare April 15, 2024 08:51

ruzniaievdm requested a review from KristinAoki April 15, 2024 09:23

KristinAoki approved these changes Apr 22, 2024

View reviewed changes

KristinAoki merged commit 907ce50 into openedx:master Apr 23, 2024
4 checks passed

ruzniaievdm mentioned this pull request May 8, 2024

Group Config Page - location counts not displaying #981

Closed

GlugovGrGlib mentioned this pull request Jun 14, 2024

New Content Grouping Page (Studio MFE conversion) openedx/platform-roadmap#318

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [FC-0044] group configurations MFE page #929

feat: [FC-0044] group configurations MFE page #929

ruzniaievdm commented Mar 31, 2024 •

edited

Loading

openedx-webhooks commented Mar 31, 2024 •

edited

Loading

codecov bot commented Apr 1, 2024 •

edited

Loading

open-craft-grove commented Apr 7, 2024

KristinAoki Apr 8, 2024

ruzniaievdm Apr 9, 2024

KristinAoki Apr 8, 2024

ruzniaievdm Apr 9, 2024

KristinAoki Apr 10, 2024

KristinAoki May 8, 2024

KristinAoki Apr 8, 2024

KristinAoki May 8, 2024

KristinAoki Apr 8, 2024

ruzniaievdm Apr 9, 2024

KristinAoki Apr 10, 2024

open-craft-grove commented Apr 15, 2024

open-craft-grove commented Apr 15, 2024

openedx-webhooks commented Apr 23, 2024

feat: [FC-0044] group configurations MFE page #929

feat: [FC-0044] group configurations MFE page #929

Conversation

ruzniaievdm commented Mar 31, 2024 • edited Loading

Issue (figma url contains)

Learn about group configurations:

Testing instructions

Demo

Content Group

Enrollment Track group

Experiment configurations

openedx-webhooks commented Mar 31, 2024 • edited Loading

codecov bot commented Apr 1, 2024 • edited Loading

Codecov Report

open-craft-grove commented Apr 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

open-craft-grove commented Apr 15, 2024

open-craft-grove commented Apr 15, 2024

openedx-webhooks commented Apr 23, 2024

ruzniaievdm commented Mar 31, 2024 •

edited

Loading

openedx-webhooks commented Mar 31, 2024 •

edited

Loading

codecov bot commented Apr 1, 2024 •

edited

Loading