Refactor LearnerCreditAllocationTable #1019
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1019 +/- ##
==========================================
- Coverage 82.88% 82.88% -0.01%
==========================================
Files 401 401
Lines 8730 8727 -3
Branches 1801 1800 -1
==========================================
- Hits 7236 7233 -3
Misses 1455 1455
Partials 39 39
☔ View full report in Codecov by Sentry. |
|
The mocks show that the content title was more bolded or larger than the email, but in your screenshot both the title and email share the same typeface. Was this intentionally deferred? https://2u-internal.atlassian.net/browse/ENT-7431 |
src/components/learner-credit-management/LearnerCreditAllocationTable.jsx
Outdated
Show resolved
Hide resolved
src/components/learner-credit-management/LearnerCreditAllocationTable.jsx
Show resolved
Hide resolved
emrosarioa
force-pushed
the
ea/ent-7512
branch
from
b0f55c9
to
2920f84
Compare
src/components/learner-credit-management/LearnerCreditAllocationTable.jsx
Outdated
Show resolved
Hide resolved
emrosarioa
force-pushed
the
ea/ent-7512
branch
2 times, most recently
from
3e07dfb
to
b7bc888
Compare
emrosarioa
force-pushed
the
ea/ent-7512
branch
from
b7bc888
to
6c5cd39
Compare
| @@ -63,18 +63,15 @@ const applySortByToOptions = (sortBy, options) => { | |||
| }; | |||
|
|
|||
| const applyFiltersToOptions = (filters, options) => { | |||
| const userSearchQuery = filters?.find(filter => filter.id === 'userEmail')?.value; | |||
| const courseTitleSearchQuery = filters?.find(filter => filter.id === 'courseTitle')?.value; | |||
| const courseProductLineSearchQuery = filters?.find(filter => filter.id === 'courseProductLine')?.value; | |||
There was a problem hiding this comment.
Follow-up from Slack conversation:
[clarification] in the current state of the (LCM) world, is the courseProductLine filter being used?
const courseProductLineSearchQuery = filters?.find(filter => filter.id === 'courseProductLine')?.value;
[Emily] probably not.
[Adam] even if not used in the current state, since the "Product" column you're keeping in for now isn't exposed in the filters (i.e., disableFilters: true), Markhors next iteration probably will be using it again lolol. So maybe we just leave it in for now with that assumption it will be used again in the near future.
| if (courseTitleSearchQuery) { | ||
| Object.assign(options, { searchCourse: courseTitleSearchQuery }); | ||
| } | ||
| const searchQuery = filters?.find(filter => filter.id.toLowerCase() === 'enrollment details')?.value; |
There was a problem hiding this comment.
[curious/clarification] Do you happen to know where this filter id of "enrollment details" (with the space) is coming from? My understanding was that the filter.id was based on the accessor for the column (e.g., like courseProductLine above). But the accessor for the "Enrollment details" column is an object (first time seeing this).
There was a problem hiding this comment.
Yeah I was surprised as well. It appears that if the accessor is a function (which in this case, it is), it defaults the filter.id to the Header of the column.
Refactor the
LearnerCreditAllocationTableto have a singleEnrollment Detailscolumn for both the user email or the course title, and also a single input for filtering the search results.This uses the API filtering implementation from here: openedx/edx-enterprise-data#388
Previous table:
Table after changes:
For all changes
Only if submitting a visual change