From 9174fd5ce803d34626281eea4ec0576fd2f90394 Mon Sep 17 00:00:00 2001 From: Abdullah Waheed Date: Tue, 7 Jun 2022 21:00:08 +0500 Subject: [PATCH 01/33] refactor: updated columns for table deprecations --- .../Admin/__snapshots__/Admin.test.jsx.snap | 20 +- .../ManageCodesTab.test.jsx.snap | 4 +- .../CodeSearchResultsTable.jsx | 4 +- .../CodeSearchResults.test.jsx.snap | 8 +- .../CompletedLearnersTable/index.jsx | 8 +- src/components/CouponDetails/index.jsx | 4 +- ...rnersForInactiveCoursesTable.test.jsx.snap | 408 ++-- .../index.jsx | 16 +- src/components/EnrollmentsTable/index.jsx | 36 +- src/components/EnterpriseList/index.jsx | 4 +- .../LearnerActivityTable.test.jsx.snap | 1889 ++++++++--------- src/components/LearnerActivityTable/index.jsx | 36 +- .../PastWeekPassedLearnersTable.test.jsx.snap | 283 +-- .../PastWeekPassedLearnersTable/index.jsx | 12 +- .../RegisteredLearnersTable/index.jsx | 8 +- .../RequestDateCell.test.jsx.snap | 2 +- ...ubsidyRequestManagementTable.test.jsx.snap | 6 +- src/components/TableComponent/index.jsx | 5 +- 18 files changed, 1320 insertions(+), 1433 deletions(-) diff --git a/src/components/Admin/__snapshots__/Admin.test.jsx.snap b/src/components/Admin/__snapshots__/Admin.test.jsx.snap index 4caf8e4944..95a256fd1a 100644 --- a/src/components/Admin/__snapshots__/Admin.test.jsx.snap +++ b/src/components/Admin/__snapshots__/Admin.test.jsx.snap @@ -661,7 +661,7 @@ exports[` renders correctly with dashboard analytics data renders # cou className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - July 31, 2018 + August 1, 2018
renders correctly with dashboard analytics data renders # of className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - July 31, 2018 + August 1, 2018
renders correctly with dashboard analytics data renders # of className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - July 31, 2018 + August 1, 2018
renders correctly with dashboard analytics data renders colla className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - July 31, 2018 + August 1, 2018
renders correctly with dashboard analytics data renders full className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - July 31, 2018 + August 1, 2018
renders correctly with dashboard analytics data renders inact className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - July 31, 2018 + August 1, 2018
renders correctly with dashboard analytics data renders inact className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - July 31, 2018 + August 1, 2018
renders correctly with dashboard analytics data renders learn className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - July 31, 2018 + August 1, 2018
renders correctly with dashboard analytics data renders regis className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - July 31, 2018 + August 1, 2018
renders correctly with dashboard analytics data renders top a className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - July 31, 2018 + August 1, 2018
- January 3, 2019 + January 4, 2019
- January 3, 2019 + January 4, 2019
{ } else if (isValidEmail(searchQuery) !== undefined && assignedToColumnIndex === -1) { // Add "Assigned To" column if it doesn't already exist tableColumns.splice(4, 0, { - label: 'Assigned To', - key: 'assignedTo', + Header: 'Assigned To', + accessor: 'assignedTo', }); } return tableColumns; diff --git a/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap b/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap index 1f5ecdc033..da6b36d35a 100644 --- a/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap +++ b/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap @@ -48,7 +48,7 @@ exports[` basic rendering should render empty table data 1` Close search results @@ -71,7 +71,7 @@ exports[` basic rendering should render empty table data 1`
basic rendering should render error 1`] = ` Close search results @@ -158,7 +158,7 @@ exports[` basic rendering should render error 1`] = `
{ const tableColumns = [ { - label: 'Email', - key: 'user_email', + Header: 'Email', + accessor: 'user_email', columnSortable: true, }, { - label: 'Total Course Completed Count', - key: 'completed_courses', + Header: 'Total Course Completed Count', + accessor: 'completed_courses', columnSortable: true, }, ]; diff --git a/src/components/CouponDetails/index.jsx b/src/components/CouponDetails/index.jsx index 6e3e38abe7..04e28c5cfc 100644 --- a/src/components/CouponDetails/index.jsx +++ b/src/components/CouponDetails/index.jsx @@ -84,7 +84,7 @@ class CouponDetails extends React.Component { getNewColumns(selectedToggle) { const selectColumn = { - label: ( + Header: ( ), - key: 'select', + accessor: 'select', }; switch (selectedToggle) { diff --git a/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap b/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap index 347187a232..225c4153a1 100644 --- a/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap +++ b/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap @@ -44,222 +44,224 @@ exports[`EnrolledLearnersForInactiveCoursesTable renders enrolled learners for i
- - - - - - - click to sort - - - - - - - + + + + + + + - - click to sort - - - - - - - + + + + - - click to sort - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + +
- - + + + +
-
- + + + Total Course Enrollment Count + + + + + + Total Completed Courses Count + + + + + + Last Activity Date + + +
- + 2 + + 1 + + June 23, 2017 +
- - test_user_1@example.com - - - 2 - - 1 - - June 23, 2017 -
- - test_user_2@example.com - - - 5 - - 5 - - January 15, 2018 -
- - test_user_3@example.com - - - 6 - - 4 - + + test_user_2@example.com + + + 5 + + 5 + + January 15, 2018 +
+ + test_user_3@example.com + + + 6 + + 4 + + November 18, 2017 +
+
+
- November 18, 2017 - - - - +
+ Showing + 3 + of + 3 + . +
+
+
+ + diff --git a/src/components/EnrolledLearnersForInactiveCoursesTable/index.jsx b/src/components/EnrolledLearnersForInactiveCoursesTable/index.jsx index 9f4ca1e817..42ba57c441 100644 --- a/src/components/EnrolledLearnersForInactiveCoursesTable/index.jsx +++ b/src/components/EnrolledLearnersForInactiveCoursesTable/index.jsx @@ -7,23 +7,23 @@ import EnterpriseDataApiService from '../../data/services/EnterpriseDataApiServi const EnrolledLearnersForInactiveCoursesTable = () => { const tableColumns = [ { - label: 'Email', - key: 'user_email', + Header: 'Email', + accessor: 'user_email', columnSortable: true, }, { - label: 'Total Course Enrollment Count', - key: 'enrollment_count', + Header: 'Total Course Enrollment Count', + accessor: 'enrollment_count', columnSortable: true, }, { - label: 'Total Completed Courses Count', - key: 'course_completion_count', + Header: 'Total Completed Courses Count', + accessor: 'course_completion_count', columnSortable: true, }, { - label: 'Last Activity Date', - key: 'last_activity_date', + Header: 'Last Activity Date', + accessor: 'last_activity_date', columnSortable: true, }, ]; diff --git a/src/components/EnrollmentsTable/index.jsx b/src/components/EnrollmentsTable/index.jsx index e53a63b5be..e13ed6eba9 100644 --- a/src/components/EnrollmentsTable/index.jsx +++ b/src/components/EnrollmentsTable/index.jsx @@ -7,48 +7,48 @@ import EnterpriseDataApiService from '../../data/services/EnterpriseDataApiServi const EnrollmentsTable = () => { const enrollmentTableColumns = [ { - label: 'Email', - key: 'user_email', + Header: 'Email', + accessor: 'user_email', columnSortable: true, }, { - label: 'Course Title', - key: 'course_title', + Header: 'Course Title', + accessor: 'course_title', columnSortable: true, }, { - label: 'Course Price', - key: 'course_list_price', + Header: 'Course Price', + accessor: 'course_list_price', columnSortable: true, }, { - label: 'Start Date', - key: 'course_start_date', + Header: 'Start Date', + accessor: 'course_start_date', columnSortable: true, }, { - label: 'End Date', - key: 'course_end_date', + Header: 'End Date', + accessor: 'course_end_date', columnSortable: true, }, { - label: 'Passed Date', - key: 'passed_date', + Header: 'Passed Date', + accessor: 'passed_date', columnSortable: true, }, { - label: 'Current Grade', - key: 'current_grade', + Header: 'Current Grade', + accessor: 'current_grade', columnSortable: true, }, { - label: 'Progress Status', - key: 'progress_status', + Header: 'Progress Status', + accessor: 'progress_status', columnSortable: true, }, { - label: 'Last Activity Date', - key: 'last_activity_date', + Header: 'Last Activity Date', + accessor: 'last_activity_date', columnSortable: true, }, ]; diff --git a/src/components/EnterpriseList/index.jsx b/src/components/EnterpriseList/index.jsx index 990ab6f794..1c8aa519f0 100644 --- a/src/components/EnterpriseList/index.jsx +++ b/src/components/EnterpriseList/index.jsx @@ -103,8 +103,8 @@ class EnterpriseList extends React.Component { const { error, loading, enterpriseList } = this.props; const columns = [ { - label: 'Enterprise', - key: 'link', + Header: 'Enterprise', + accessor: 'link', }, ]; diff --git a/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap b/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap index 4f4a143990..0d124a790d 100644 --- a/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap +++ b/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap @@ -13,380 +13,311 @@ exports[`LearnerActivityTable renders active learners table correctly 1`] = `
- - - - - - - - - - - - click to sort - - - - - - - + + + + + + + + + + + + - - click to sort - - - - - - - + + + + + + + + + - - click to sort - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
- - - - - - - - - - - - + + + +
-
- + + + Course Title + + + + + + Course Price + + + + + + Start Date + + + + + + End Date + + + + + + Passed Date + + + + + + Current Grade + + + + + + Progress Status + + + + + + Last Activity Date + + +
- + Dive into ReactJS + + $200 + + October 22, 2017 + + May 13, 2018 + + September 23, 2018 + + 66% + + Failed + + September 22, 2018 +
- - awesome.me@example.com - - - Dive into ReactJS - - $200 - - October 21, 2017 - - May 13, 2018 - - September 23, 2018 - - 66% - - Failed - - September 22, 2018 -
- - new@example.com - - - Redux with ReactJS - - $200 - - October 21, 2017 - - May 13, 2018 - - September 22, 2018 - - 80% - - Passed - - September 25, 2018 -
+ + + new@example.com + + + + Redux with ReactJS + + + $200 + + + October 22, 2017 + + + May 13, 2018 + + + September 22, 2018 + + + 80% + + + Passed + + + September 25, 2018 + + + + +
+
+
+ Showing + 2 + of + 2 + . +
+
+ + + @@ -532,342 +463,311 @@ exports[`LearnerActivityTable renders inactive past month learners table correct
- - - - - - - - - - - click to sort - - - - - - - + + + + + + + + + + + + - - click to sort - - - - - - - + + + + + + + + + - - click to sort - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
- - - - - - - - - - + + + +
-
- + + + Course Title + + + + + + Course Price + + + + + + Start Date + + + + + + End Date + + + + + + Passed Date + + + + + + Current Grade + + + + + + Progress Status + + + + + + Last Activity Date + + +
- + Dive into ReactJS + + $200 + + October 22, 2017 + + May 13, 2018 + + September 23, 2018 + + 66% + + Failed + + September 22, 2018 +
- - awesome.me@example.com - - - Dive into ReactJS - - $200 - - October 21, 2017 - - May 13, 2018 - - 66% - - Failed - - September 22, 2018 -
- - new@example.com - - - Redux with ReactJS - - $200 - - October 21, 2017 - - May 13, 2018 - - 80% - - Passed - - September 25, 2018 -
+ + + new@example.com + + + + Redux with ReactJS + + + $200 + + + October 22, 2017 + + + May 13, 2018 + + + September 22, 2018 + + + 80% + + + Passed + + + September 25, 2018 + + + + +
+
+
+ Showing + 2 + of + 2 + . +
+
+ + + @@ -982,342 +882,311 @@ exports[`LearnerActivityTable renders inactive past week learners table correctl
- - - - - - - - - - - click to sort - - - - - - - + + + + + + + + + + + + - - click to sort - - - - - - - + + + + + + + + + - - click to sort - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
- - - - - - - - - - + + + +
-
- + + + Course Title + + + + + + Course Price + + + + + + Start Date + + + + + + End Date + + + + + + Passed Date + + + + + + Current Grade + + + + + + Progress Status + + + + + + Last Activity Date + + +
- + Dive into ReactJS + + $200 + + October 22, 2017 + + May 13, 2018 + + September 23, 2018 + + 66% + + Failed + + September 22, 2018 +
- - awesome.me@example.com - - - Dive into ReactJS - - $200 - - October 21, 2017 - - May 13, 2018 - - 66% - - Failed - - September 22, 2018 -
- - new@example.com - - - Redux with ReactJS - - $200 - - October 21, 2017 - - May 13, 2018 - - 80% - - Passed - - September 25, 2018 -
+ + + new@example.com + + + + Redux with ReactJS + + + $200 + + + October 22, 2017 + + + May 13, 2018 + + + September 22, 2018 + + + 80% + + + Passed + + + September 25, 2018 + + + + +
+
+
+ Showing + 2 + of + 2 + . +
+
+ + + diff --git a/src/components/LearnerActivityTable/index.jsx b/src/components/LearnerActivityTable/index.jsx index 23a95e5756..6fb3a1de34 100644 --- a/src/components/LearnerActivityTable/index.jsx +++ b/src/components/LearnerActivityTable/index.jsx @@ -10,48 +10,48 @@ class LearnerActivityTable extends React.Component { const { activity } = this.props; const tableColumns = [ { - label: 'Email', - key: 'user_email', + Header: 'Email', + accessor: 'user_email', columnSortable: true, }, { - label: 'Course Title', - key: 'course_title', + Header: 'Course Title', + accessor: 'course_title', columnSortable: true, }, { - label: 'Course Price', - key: 'course_list_price', + Header: 'Course Price', + accessor: 'course_list_price', columnSortable: true, }, { - label: 'Start Date', - key: 'course_start_date', + Header: 'Start Date', + accessor: 'course_start_date', columnSortable: true, }, { - label: 'End Date', - key: 'course_end_date', + Header: 'End Date', + accessor: 'course_end_date', columnSortable: true, }, { - label: 'Passed Date', - key: 'passed_date', + Header: 'Passed Date', + accessor: 'passed_date', columnSortable: true, }, { - label: 'Current Grade', - key: 'current_grade', + Header: 'Current Grade', + accessor: 'current_grade', columnSortable: true, }, { - label: 'Progress Status', - key: 'progress_status', + Header: 'Progress Status', + accessor: 'progress_status', columnSortable: true, }, { - label: 'Last Activity Date', - key: 'last_activity_date', + Header: 'Last Activity Date', + accessor: 'last_activity_date', columnSortable: true, }, ]; diff --git a/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap b/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap index 5bd0b51726..c5d603fb8b 100644 --- a/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap +++ b/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap @@ -13,152 +13,167 @@ exports[`PastWeekPassedLearnersTable renders table correctly 1`] = `
- - - - - - - click to sort - - - - - - - + + + + + + - - click to sort - - - - - - - - - - - - - - - - - - + + + + + + + + + +
- - + + + +
-
- + + + Course Title + + + + + + Passed Date + + +
- - awesome.me@example.com - - - Dive into ReactJS - - September 23, 2018 -
- - new@example.com - - - Redux with ReactJS - + + awesome.me@example.com + + + Dive into ReactJS + + September 23, 2018 +
+ + new@example.com + + + Redux with ReactJS + + September 22, 2018 +
+
+
- September 22, 2018 - - - - +
+ Showing + 2 + of + 2 + . +
+
+ + + diff --git a/src/components/PastWeekPassedLearnersTable/index.jsx b/src/components/PastWeekPassedLearnersTable/index.jsx index 72c9dd0415..93ecaa8bd0 100644 --- a/src/components/PastWeekPassedLearnersTable/index.jsx +++ b/src/components/PastWeekPassedLearnersTable/index.jsx @@ -7,18 +7,18 @@ import { formatTimestamp } from '../../utils'; const PastWeekPassedLearnersTable = () => { const tableColumns = [ { - label: 'Email', - key: 'user_email', + Header: 'Email', + accessor: 'user_email', columnSortable: true, }, { - label: 'Course Title', - key: 'course_title', + Header: 'Course Title', + accessor: 'course_title', columnSortable: true, }, { - label: 'Passed Date', - key: 'passed_date', + Header: 'Passed Date', + accessor: 'passed_date', columnSortable: true, }, ]; diff --git a/src/components/RegisteredLearnersTable/index.jsx b/src/components/RegisteredLearnersTable/index.jsx index e2e60636f2..0a357107dd 100644 --- a/src/components/RegisteredLearnersTable/index.jsx +++ b/src/components/RegisteredLearnersTable/index.jsx @@ -8,13 +8,13 @@ import EnterpriseDataApiService from '../../data/services/EnterpriseDataApiServi const RegisteredLearnersTable = () => { const tableColumns = [ { - label: 'Email', - key: 'user_email', + Header: 'Email', + accessor: 'user_email', columnSortable: true, }, { - label: 'Account Created', - key: 'lms_user_created', + Header: 'Account Created', + accessor: 'lms_user_created', columnSortable: true, }, ]; diff --git a/src/components/SubsidyRequestManagementTable/tests/__snapshots__/RequestDateCell.test.jsx.snap b/src/components/SubsidyRequestManagementTable/tests/__snapshots__/RequestDateCell.test.jsx.snap index edaad46951..75e1c90a0a 100644 --- a/src/components/SubsidyRequestManagementTable/tests/__snapshots__/RequestDateCell.test.jsx.snap +++ b/src/components/SubsidyRequestManagementTable/tests/__snapshots__/RequestDateCell.test.jsx.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`RequestDateCell renders as expected 1`] = `"December 3, 2019"`; +exports[`RequestDateCell renders as expected 1`] = `"December 4, 2019"`; diff --git a/src/components/SubsidyRequestManagementTable/tests/__snapshots__/SubsidyRequestManagementTable.test.jsx.snap b/src/components/SubsidyRequestManagementTable/tests/__snapshots__/SubsidyRequestManagementTable.test.jsx.snap index 59a94d0637..c920c65ddc 100644 --- a/src/components/SubsidyRequestManagementTable/tests/__snapshots__/SubsidyRequestManagementTable.test.jsx.snap +++ b/src/components/SubsidyRequestManagementTable/tests/__snapshots__/SubsidyRequestManagementTable.test.jsx.snap @@ -196,7 +196,7 @@ exports[`SubsidyRequestManagementTable renders data in a table as expected 1`] = className="pgn__data-table-cell-wrap" role="cell" > - December 3, 2019 + December 4, 2019 - December 3, 2020 + December 4, 2020 - December 3, 2021 + December 4, 2021 {loading && }
- From 6250d81d201fd88d296b5b0001bae06baf384d1a Mon Sep 17 00:00:00 2001 From: Abdullah Waheed Date: Wed, 8 Jun 2022 14:58:54 +0500 Subject: [PATCH 02/33] refactor: updated unit tests for Datatable changes --- .../Admin/__snapshots__/Admin.test.jsx.snap | 20 +- .../ManageCodesTab.test.jsx.snap | 4 +- .../CodeSearchResultsTable.jsx | 24 +- .../CodeSearchResults.test.jsx.snap | 873 +++++++++++------- src/components/CouponDetails/constants.js | 32 +- src/components/CouponDetails/index.jsx | 4 +- src/components/CouponDetails/index.test.jsx | 4 +- .../EnrolledLearnersTable/index.jsx | 12 +- .../LearnerActivityTable.test.jsx | 6 + .../LearnerActivityTable.test.jsx.snap | 12 +- .../RequestDateCell.test.jsx.snap | 2 +- ...ubsidyRequestManagementTable.test.jsx.snap | 6 +- .../CouponDetails/CouponDetails.test.jsx | 50 +- 13 files changed, 624 insertions(+), 425 deletions(-) diff --git a/src/components/Admin/__snapshots__/Admin.test.jsx.snap b/src/components/Admin/__snapshots__/Admin.test.jsx.snap index 95a256fd1a..4caf8e4944 100644 --- a/src/components/Admin/__snapshots__/Admin.test.jsx.snap +++ b/src/components/Admin/__snapshots__/Admin.test.jsx.snap @@ -661,7 +661,7 @@ exports[` renders correctly with dashboard analytics data renders # cou className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - August 1, 2018 + July 31, 2018
renders correctly with dashboard analytics data renders # of className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - August 1, 2018 + July 31, 2018
renders correctly with dashboard analytics data renders # of className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - August 1, 2018 + July 31, 2018
renders correctly with dashboard analytics data renders colla className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - August 1, 2018 + July 31, 2018
renders correctly with dashboard analytics data renders full className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - August 1, 2018 + July 31, 2018
renders correctly with dashboard analytics data renders inact className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - August 1, 2018 + July 31, 2018
renders correctly with dashboard analytics data renders inact className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - August 1, 2018 + July 31, 2018
renders correctly with dashboard analytics data renders learn className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - August 1, 2018 + July 31, 2018
renders correctly with dashboard analytics data renders regis className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - August 1, 2018 + July 31, 2018
renders correctly with dashboard analytics data renders top a className="col-12 col-md-6 col-xl-4 pt-1 pb-3" > Showing data as of - August 1, 2018 + July 31, 2018
- January 4, 2019 + January 3, 2019
- January 4, 2019 + January 3, 2019
basic rendering should render empty table data 1` Close search results @@ -71,7 +71,7 @@ exports[` basic rendering should render empty table data 1`
basic rendering should render error 1`] = ` Close search results @@ -158,7 +158,7 @@ exports[` basic rendering should render error 1`] = `
basic rendering should render table data 1`] = `
-
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
- Coupon Batch - - Code - - Redeemed - - Redemption Date - - Course Title - - Actions -
- Test Coupon Name - - Y7XS3OGG7WB7KQ5R - - - - has been redeemed - - - August 28, 2019 - - Test Course Title - - - -
- Test Coupon Name 2 - - CMX9N1LPGTUSL7DU - - - - - - - - - - - - -
- Test Coupon Name - - Y7XS3OGG7WB7KQ5R - +
+ Showing + 4 + of + 4 + . +
+ +
+
+
+
+
+
+
- - - has been redeemed - -
- August 30, 2019 - - Test Course Title - - - -
- Test Coupon Name - - FAG2LVLNHAKIXQ0Q - - - - - - - - - - +
+ + + Coupon Batch + + + + + + Code + + + + + + Redeemed + + + + + + Redemption Date + + + + + + Course Title + + + + + + Actions + + +
+ Test Coupon Name + + Y7XS3OGG7WB7KQ5R + + + + has been redeemed + + + August 28, 2019 + + Test Course Title + + - +
+ Test Coupon Name 2 + + CMX9N1LPGTUSL7DU + + - + + - + + - + + - +
+ Test Coupon Name + + Y7XS3OGG7WB7KQ5R + + + + has been redeemed + + + August 30, 2019 + + Test Course Title + + - +
+ Test Coupon Name + + FAG2LVLNHAKIXQ0Q + + - + + - + + - + + - +
+
+
- - - - - - +
+ Showing + 4 + of + 4 + . +
+
+ + + @@ -974,126 +1069,224 @@ exports[` basic rendering should render table data when sea
- - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + +
- Coupon Batch - - Code - - Redeemed - - Redemption Date - - Assigned To - - Course Title - - Actions -
- Test Coupon Name - - FAG2LVLNHAKIXQ0Q - - - - - - - - test@test.com - - - - - - | -
+ + + Coupon Batch + + + + + + Code + + + + + + Redeemed + + + + + + Redemption Date + + + + + + Assigned To + + + + + + Course Title + + + + + + Actions + + +
+ Test Coupon Name + + FAG2LVLNHAKIXQ0Q + + - + + - + + test@test.com + + - + + + | + +
+
+
+
+ Showing + 1 + of + 1 + . +
+
+ + + diff --git a/src/components/CouponDetails/constants.js b/src/components/CouponDetails/constants.js index 180decda7f..5c4e566a18 100644 --- a/src/components/CouponDetails/constants.js +++ b/src/components/CouponDetails/constants.js @@ -81,36 +81,36 @@ export const DETAILS_TEXT = { export const COLUMNS = { redemptions: { - label: 'Redemptions', - key: 'redemptions', + Header: 'Redemptions', + accessor: 'redemptions', }, code: { - label: 'Code', - key: 'code', + Header: 'Code', + accessor: 'code', }, assignmentsRemaining: { - label: 'Assignments remaining', - key: 'assignments_remaining', + Header: 'Assignments remaining', + accessor: 'assignments_remaining', }, actions: { - label: 'Actions', - key: 'actions', + Header: 'Actions', + accessor: 'actions', }, lastReminderDate: { - label: 'Last reminder date', - key: 'last_reminder_date', + Header: 'Last reminder date', + accessor: 'last_reminder_date', }, assignmentDate: { - label: 'Assignment date', - key: 'assignment_date', + Header: 'Assignment date', + accessor: 'assignment_date', }, assignedTo: { - label: 'Assigned to', - key: 'assigned_to', + Header: 'Assigned to', + accessor: 'assigned_to', }, redeemedBy: { - label: 'Redeemed by', - key: 'assigned_to', + Header: 'Redeemed by', + accessor: 'assigned_to', }, }; diff --git a/src/components/CouponDetails/index.jsx b/src/components/CouponDetails/index.jsx index 04e28c5cfc..6cd16f18b4 100644 --- a/src/components/CouponDetails/index.jsx +++ b/src/components/CouponDetails/index.jsx @@ -234,7 +234,7 @@ class CouponDetails extends React.Component { const selectColumn = tableColumns.shift(); - selectColumn.label = React.cloneElement(selectColumn.label, { + selectColumn.Header = React.cloneElement(selectColumn.Header, { checked: allCodesForPageSelected, className: hasPartialSelection ? ['mixed'] : [], }); @@ -244,7 +244,7 @@ class CouponDetails extends React.Component { // attribute appropriately. // // TODO: Paragon now has an IndeterminateCheckbox that can be used here. - const selectAllCheckBoxRef = selectColumn.label.ref && selectColumn.label.ref.current; + const selectAllCheckBoxRef = selectColumn.Header.ref && selectColumn.Header.ref.current; const selectAllCheckBoxDOM = ( selectAllCheckBoxRef && document.getElementById(selectAllCheckBoxRef.props.id) ); diff --git a/src/components/CouponDetails/index.test.jsx b/src/components/CouponDetails/index.test.jsx index b2edf62f15..2e64177cb5 100644 --- a/src/components/CouponDetails/index.test.jsx +++ b/src/components/CouponDetails/index.test.jsx @@ -157,8 +157,8 @@ describe('CouponDetails component', () => { it('renders the unassigned table by default', () => { renderWithRouter(); expect(screen.getByText(COUPON_FILTERS.unassigned.label)).toBeInTheDocument(); - DEFAULT_TABLE_COLUMNS.unassigned.forEach(({ label }) => { - expect(screen.getByText(label)).toBeInTheDocument(); + DEFAULT_TABLE_COLUMNS.unassigned.forEach(({ Header }) => { + expect(screen.getByText(Header)).toBeInTheDocument(); }); }); it('renders with error state', () => { diff --git a/src/components/EnrolledLearnersTable/index.jsx b/src/components/EnrolledLearnersTable/index.jsx index 65e0b1d4fc..1dc1ff3eb2 100644 --- a/src/components/EnrolledLearnersTable/index.jsx +++ b/src/components/EnrolledLearnersTable/index.jsx @@ -7,18 +7,18 @@ import EnterpriseDataApiService from '../../data/services/EnterpriseDataApiServi const EnrolledLearnersTable = () => { const tableColumns = [ { - label: 'Email', - key: 'user_email', + Header: 'Email', + accessor: 'user_email', columnSortable: true, }, { - label: 'Account Created', - key: 'lms_user_created', + Header: 'Account Created', + accessor: 'lms_user_created', columnSortable: true, }, { - label: 'Total Course Enrollment Count', - key: 'enrollment_count', + Header: 'Total Course Enrollment Count', + accessor: 'enrollment_count', columnSortable: true, }, ]; diff --git a/src/components/LearnerActivityTable/LearnerActivityTable.test.jsx b/src/components/LearnerActivityTable/LearnerActivityTable.test.jsx index 8d03a190c3..4b4c0ea2be 100644 --- a/src/components/LearnerActivityTable/LearnerActivityTable.test.jsx +++ b/src/components/LearnerActivityTable/LearnerActivityTable.test.jsx @@ -212,6 +212,7 @@ describe('LearnerActivityTable', () => { 'Course Price', 'Start Date', 'End Date', + 'Passed Date', 'Current Grade', 'Progress Status', 'Last Activity Date', @@ -223,6 +224,7 @@ describe('LearnerActivityTable', () => { '$200', 'October 21, 2017', 'May 13, 2018', + 'September 23, 2018', '66%', 'Failed', 'September 22, 2018', @@ -233,6 +235,7 @@ describe('LearnerActivityTable', () => { '$200', 'October 21, 2017', 'May 13, 2018', + 'September 22, 2018', '80%', 'Passed', 'September 25, 2018', @@ -251,6 +254,7 @@ describe('LearnerActivityTable', () => { 'Course Price', 'Start Date', 'End Date', + 'Passed Date', 'Current Grade', 'Progress Status', 'Last Activity Date', @@ -262,6 +266,7 @@ describe('LearnerActivityTable', () => { '$200', 'October 21, 2017', 'May 13, 2018', + 'September 23, 2018', '66%', 'Failed', 'September 22, 2018', @@ -272,6 +277,7 @@ describe('LearnerActivityTable', () => { '$200', 'October 21, 2017', 'May 13, 2018', + 'September 22, 2018', '80%', 'Passed', 'September 25, 2018', diff --git a/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap b/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap index 0d124a790d..dd692531dc 100644 --- a/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap +++ b/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap @@ -205,7 +205,7 @@ exports[`LearnerActivityTable renders active learners table correctly 1`] = ` className="pgn__data-table-cell-wrap" role="cell" > - October 22, 2017 + October 21, 2017 - October 22, 2017 + October 21, 2017 - October 22, 2017 + October 21, 2017 - October 22, 2017 + October 21, 2017 - October 22, 2017 + October 21, 2017 - October 22, 2017 + October 21, 2017 - December 4, 2019 + December 3, 2019 - December 4, 2020 + December 3, 2020 - December 4, 2021 + December 3, 2021 { }); renderWithRouter(); expect(screen.getByText(COUPON_FILTERS.unassigned.label)).toBeInTheDocument(); - DEFAULT_TABLE_COLUMNS.unassigned.forEach(({ label }) => { - expect(screen.getByText(label)).toBeInTheDocument(); + DEFAULT_TABLE_COLUMNS.unassigned.forEach(({ Header }) => { + expect(screen.getByText(Header)).toBeInTheDocument(); }); }); @@ -203,8 +203,8 @@ describe('CouponDetails container', () => { renderWithRouter(); userEvent.selectOptions(screen.getByLabelText('Filter by code status'), filterType); - DEFAULT_TABLE_COLUMNS[filterType].forEach(({ label }) => { - expect(screen.getByText(label)).toBeInTheDocument(); + DEFAULT_TABLE_COLUMNS[filterType].forEach(({ Header }) => { + expect(screen.getByText(Header)).toBeInTheDocument(); }); }); @@ -390,14 +390,14 @@ describe('CouponDetails container', () => { describe('modals', () => { let spy; - const openModalByActionButton = ({ key }) => { - const actionButton = wrapper.find('table').find('button').find(`.${key}-btn`); + const openModalByActionButton = ({ accessor }) => { + const actionButton = wrapper.find('table').find('button').find(`.${accessor}-btn`); actionButton.simulate('click'); }; - const testModalActionButton = ({ key, label }) => { - const actionButton = wrapper.find('table').find('button').find(`.${key}-btn`); - expect(actionButton.text()).toEqual(label); + const testModalActionButton = ({ accessor, Header }) => { + const actionButton = wrapper.find('table').find('button').find(`.${accessor}-btn`); + expect(actionButton.text()).toEqual(Header); actionButton.simulate('click'); }; @@ -424,22 +424,22 @@ describe('CouponDetails container', () => { it('sets remind modal state on Remind button click', () => { testModalActionButton({ - key: 'remind', - label: 'Remind', + accessor: 'remind', + Header: 'Remind', }); }); it('sets revoke modal state on Revoke button click', () => { testModalActionButton({ - key: 'revoke', - label: 'Revoke', + accessor: 'revoke', + Header: 'Revoke', }); }); it('sets assignment modal state on Assign button click', () => { testModalActionButton({ - key: 'assignment', - label: 'Assign', + accessor: 'assignment', + Header: 'Assign', }); // TODO: The remind/revoke buttons now manage their modal state in their // own components, so we only need to worry about the `assign` action now. @@ -449,8 +449,8 @@ describe('CouponDetails container', () => { it('shows correct remaining uses on assignment modal', () => { testModalActionButton({ - key: 'assignment', - label: 'Assign', + accessor: 'assignment', + Header: 'Assign', }); expect(wrapper.find('.assignment-details .code-remaining-uses').text()).toEqual('Remaining Uses: 85'); @@ -495,8 +495,8 @@ describe('CouponDetails container', () => { it('handles successful code assignment from modal', () => { openModalByActionButton({ - key: 'assignment', - label: 'Assign', + accessor: 'assignment', + Header: 'Assign', }); // fake successful code assignment @@ -520,8 +520,8 @@ describe('CouponDetails container', () => { it('handles successful code revoke from modal', () => { openModalByActionButton({ - key: 'revoke', - label: 'Revoke', + accessor: 'revoke', + Header: 'Revoke', }); // fake successful code assignment @@ -540,8 +540,8 @@ describe('CouponDetails container', () => { it('handles successful code remind from modal', () => { openModalByActionButton({ - key: 'remind', - label: 'Remind', + accessor: 'remind', + Header: 'Remind', }); // fake successful code assignment @@ -559,8 +559,8 @@ describe('CouponDetails container', () => { it('handles errors in response data for code reminder ', () => { openModalByActionButton({ - key: 'remind', - label: 'Remind', + accessor: 'remind', + Header: 'Remind', }); // fake code assignment 200 status with error in response data. From 45691d7151833f51896a6e60462c2b7ac6e0b906 Mon Sep 17 00:00:00 2001 From: Alexander Sheehan Date: Sun, 22 May 2022 23:03:32 -0400 Subject: [PATCH 03/33] feat: allowing for idp direct entry + bug fixes --- .../SettingsSSOTab/ExistingSSOConfigs.jsx | 31 ++++- .../SettingsSSOTab/SSOConfigContext.jsx | 64 ++++++--- .../settings/SettingsSSOTab/SSOStepper.jsx | 45 +++++-- .../settings/SettingsSSOTab/data/actions.js | 12 ++ .../settings/SettingsSSOTab/data/reducer.js | 9 +- .../settings/SettingsSSOTab/hooks.js | 104 ++++++++++++--- .../settings/SettingsSSOTab/index.jsx | 18 ++- .../steps/SSOConfigConfigureStep.jsx | 2 +- .../steps/SSOConfigConfigureStep.test.jsx | 36 +++++- .../steps/SSOConfigConnectStep.jsx | 5 +- .../SettingsSSOTab/steps/SSOConfigIDPStep.jsx | 121 ++++++++++++++++-- .../steps/SSOConfigIDPStep.test.jsx | 14 +- .../tests/ExistingSSOConfigs.test.jsx | 96 +++++++++++++- .../tests/NewSSOConfigForm.test.jsx | 116 ++++++++++++++++- .../tests/SSOConfigCard.test.jsx | 26 +++- .../SettingsSSOTab/tests/utils.test.js | 46 ++++++- .../settings/SettingsSSOTab/utils.js | 22 +++- 17 files changed, 668 insertions(+), 99 deletions(-) diff --git a/src/components/settings/SettingsSSOTab/ExistingSSOConfigs.jsx b/src/components/settings/SettingsSSOTab/ExistingSSOConfigs.jsx index 8cba0a4f27..2145665e15 100644 --- a/src/components/settings/SettingsSSOTab/ExistingSSOConfigs.jsx +++ b/src/components/settings/SettingsSSOTab/ExistingSSOConfigs.jsx @@ -13,10 +13,11 @@ import handleErrors from '../utils'; import LmsApiService from '../../../data/services/LmsApiService'; const errorToggleModalText = 'We were unable to toggle your configuration. Please try submitting again or contact support for help.'; -const errorDeleteModalText = 'We were unable to delete your configuration. Please try removing again or contact support for help.'; +const errorDeleteConfigModalText = 'We were unable to delete your configuration. Please try removing again or contact support for help.'; +const errorDeleteDataModalText = 'We were unable to delete your provider data. Please try removing again or contact support for help.'; const ExistingSSOConfigs = ({ - configs, refreshBool, setRefreshBool, enterpriseId, + configs, refreshBool, setRefreshBool, enterpriseId, providerData, }) => { const [errorIsOpen, openError, closeError] = useToggle(false); const [errorModalText, setErrorModalText] = useState(); @@ -28,6 +29,22 @@ const ExistingSSOConfigs = ({ setCurrentStep('idp'); }; + const deleteProviderData = async (pdid) => { + let err; + try { + await (LmsApiService.deleteProviderData(pdid, enterpriseId)); + } catch (error) { + err = handleErrors(error); + } + if (err) { + setErrorModalText(errorDeleteDataModalText); + openError(); + } else { + // changing this variable triggers the refresh + setRefreshBool(!refreshBool); + } + }; + const deleteConfig = async (id) => { let err; try { @@ -36,7 +53,7 @@ const ExistingSSOConfigs = ({ err = handleErrors(error); } if (err) { - setErrorModalText(errorDeleteModalText); + setErrorModalText(errorDeleteConfigModalText); openError(); } else { // changing this variable triggers the refresh @@ -126,7 +143,10 @@ const ExistingSSOConfigs = ({ {!config.was_valid_at && (
deleteConfig(config.id)} + onClick={() => { + deleteProviderData(providerData.id); + deleteConfig(config.id); + }} data-testid="dropdown-delete-item" > Delete @@ -159,6 +179,9 @@ const ExistingSSOConfigs = ({ ExistingSSOConfigs.propTypes = { configs: PropTypes.arrayOf(PropTypes.shape({})).isRequired, + providerData: PropTypes.shape({ + id: PropTypes.number, + }).isRequired, refreshBool: PropTypes.bool.isRequired, setRefreshBool: PropTypes.func.isRequired, enterpriseId: PropTypes.string.isRequired, diff --git a/src/components/settings/SettingsSSOTab/SSOConfigContext.jsx b/src/components/settings/SettingsSSOTab/SSOConfigContext.jsx index 3f350c17b5..466be77544 100644 --- a/src/components/settings/SettingsSSOTab/SSOConfigContext.jsx +++ b/src/components/settings/SettingsSSOTab/SSOConfigContext.jsx @@ -1,5 +1,7 @@ /* eslint-disable import/prefer-default-export */ -import { createContext, useReducer } from 'react'; +import { + createContext, useReducer, useCallback, useMemo, +} from 'react'; import PropTypes from 'prop-types'; import SSOStateReducer from './data/reducer'; import { @@ -24,7 +26,9 @@ export const SSO_INITIAL_STATE = { isComplete: false, metadataURL: '', entityID: '', - entryType: 'url', // vs directEntry + entryType: null, // url vs direct + publicKey: '', + ssoUrl: '', isDirty: false, }, serviceprovider: { @@ -50,27 +54,45 @@ const SSOConfigContextProvider = ({ children, initialState }) => { const [ssoState, dispatchSsoState] = useReducer(SSOStateReducer, initialState); // setter shortcuts - const setProviderConfig = config => dispatchSsoState(updateProviderConfig(config)); - const setCurrentError = error => dispatchSsoState(updateCurrentError(error)); - const setCurrentStep = step => dispatchSsoState(updateCurrentStep(step)); - const setIsSsoValid = valid => dispatchSsoState(updateConnectIsSsoValid(valid)); - const setInfoMessage = message => dispatchSsoState(updateInfoMessage(message)); - const setRefreshBool = refresh => dispatchSsoState(updateRefreshBool(refresh)); - const setStartTime = timeVal => dispatchSsoState(updateStartTime(timeVal)); + const setProviderConfig = useCallback((config) => { + dispatchSsoState(updateProviderConfig(config)); + }, []); + const setCurrentError = useCallback((error) => { + dispatchSsoState(updateCurrentError(error)); + }, []); + const setCurrentStep = useCallback((step) => { + dispatchSsoState(updateCurrentStep(step)); + }, []); + const setIsSsoValid = useCallback((valid) => { + dispatchSsoState(updateConnectIsSsoValid(valid)); + }, []); + const setInfoMessage = useCallback((message) => { + dispatchSsoState(updateInfoMessage(message)); + }, []); + const setRefreshBool = useCallback((refresh) => { + dispatchSsoState(updateRefreshBool(refresh)); + }, []); + const setStartTime = useCallback((timeVal) => { + dispatchSsoState(updateStartTime(timeVal)); + }, []); + + const ssoProviderValue = useMemo(() => ({ + ssoState, + dispatchSsoState, + setProviderConfig, + setCurrentError, + setCurrentStep, + setIsSsoValid, + setInfoMessage, + setRefreshBool, + setStartTime, + }), [ + ssoState, dispatchSsoState, setProviderConfig, setCurrentError, setCurrentStep, setIsSsoValid, + setInfoMessage, setRefreshBool, setStartTime, + ]); return ( - + {children} ); diff --git a/src/components/settings/SettingsSSOTab/SSOStepper.jsx b/src/components/settings/SettingsSSOTab/SSOStepper.jsx index 417d231066..55607155e4 100644 --- a/src/components/settings/SettingsSSOTab/SSOStepper.jsx +++ b/src/components/settings/SettingsSSOTab/SSOStepper.jsx @@ -29,14 +29,33 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { const [configValues, setConfigValues] = useState(null); const [connectError, setConnectError] = useState(false); const [showExitModal, setShowExitModal] = useState(false); - const [formUpdated, setFormUpdated] = React.useState(false); + const [formUpdated, setFormUpdated] = useState(false); + const [existingIdpDataEntityId, setExistingIdpDataEntityId] = useState(null); + const [existingIdpDataId, setExistingIdpDataId] = useState(null); + const [existingMetadataUrl, setExistingMetadataUrl] = useState(null); - const { metadataURL, entityID, createOrUpdateIdpRecord } = useIdpState(); - - const isIdpStepComplete = useMemo( - () => (metadataURL && isURL(metadataURL)) && (entityID && !isEmpty(entityID)), - [metadataURL, entityID], - ); + const { + entryType, + metadataURL, + entityID, + publicKey, + ssoUrl, + createOrUpdateIdpRecord, + } = useIdpState(); + + const isIdpStepComplete = useMemo(() => { + if (entryType === 'url') { + return (metadataURL && isURL(metadataURL)) && (entityID && !isEmpty(entityID)); + } + if (entryType === 'direct') { + return ( + (publicKey && !isEmpty(publicKey)) + && (entityID && !isEmpty(entityID)) + && (ssoUrl && isURL(ssoUrl)) + ); + } + return false; + }, [metadataURL, entityID, publicKey, ssoUrl, entryType]); const handleCancel = () => { setCurrentStep('idp'); @@ -65,7 +84,8 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { if (configValues !== null) { configValues.append('enterprise_customer_uuid', enterpriseId); try { - await LmsApiService.updateProviderConfig(configValues, providerConfig.id); + const response = await LmsApiService.updateProviderConfig(configValues, providerConfig.id); + setProviderConfig(response.data); } catch (error) { err = handleErrors(error); setConnectError(true); @@ -85,7 +105,11 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { - + @@ -128,6 +152,9 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { enterpriseSlug, enterpriseId, providerConfig, + existingIdpDataEntityId, + existingIdpDataId, + existingMetadataUrl, onSuccess: () => setCurrentStep('serviceprovider'), }); }} diff --git a/src/components/settings/SettingsSSOTab/data/actions.js b/src/components/settings/SettingsSSOTab/data/actions.js index 1cbe3287c7..023f0733d5 100644 --- a/src/components/settings/SettingsSSOTab/data/actions.js +++ b/src/components/settings/SettingsSSOTab/data/actions.js @@ -1,6 +1,8 @@ export const UPDATE_IDP_METADATA_URL = 'update_idp_metadata_url'; export const UPDATE_IDP_ENTRY_TYPE = 'update_idp_entry_type'; export const UPDATE_IDP_ENTITYID = 'update_idp_entityid'; +export const UPDATE_IDP_SSO_URL = 'update_sso_url'; +export const UPDATE_IDP_PUBLIC_KEY = 'update_public_key'; export const UPDATE_IDP_DIRTYSTATE = 'update_idp_dirtystate'; export const UPDATE_CURRENT_STEP = 'update_current_step'; export const UPDATE_SP_CONFIGURED = 'update_sp_configured'; @@ -29,6 +31,16 @@ export const updateEntityIDAction = entityID => ({ entityID, }); +export const updateSsoUrlAction = ssoUrl => ({ + type: UPDATE_IDP_SSO_URL, + ssoUrl, +}); + +export const updatePublicKeyAction = publicKey => ({ + type: UPDATE_IDP_PUBLIC_KEY, + publicKey, +}); + export const updateIdpDirtyState = dirtyState => ({ type: UPDATE_IDP_DIRTYSTATE, dirtyState, diff --git a/src/components/settings/SettingsSSOTab/data/reducer.js b/src/components/settings/SettingsSSOTab/data/reducer.js index 986f915f3a..4dd15a0891 100644 --- a/src/components/settings/SettingsSSOTab/data/reducer.js +++ b/src/components/settings/SettingsSSOTab/data/reducer.js @@ -2,7 +2,8 @@ import { UPDATE_CURRENT_STEP, UPDATE_IDP_ENTRY_TYPE, UPDATE_IDP_METADATA_URL, UPDATE_SP_CONFIGURED, SET_PROVIDER_CONFIG, UPDATE_IDP_ENTITYID, UPDATE_CURRENT_ERROR, CLEAR_PROVIDER_CONFIG, UPDATE_IDP_DIRTYSTATE, UPDATE_CONNECT_IN_PROGRESS, UPDATE_CONNECT_IS_SSO_VALID, - UPDATE_INFO_MESSAGE, UPDATE_CONNECT_START_TIME, UPDATE_REFRESH_BOOL, + UPDATE_INFO_MESSAGE, UPDATE_CONNECT_START_TIME, UPDATE_REFRESH_BOOL, UPDATE_IDP_SSO_URL, + UPDATE_IDP_PUBLIC_KEY, } from './actions'; const SSOStateReducer = (state, action) => { @@ -22,6 +23,12 @@ const SSOStateReducer = (state, action) => { case UPDATE_IDP_ENTITYID: { return { ...state, idp: { ...state.idp, entityID: action.entityID, isDirty: true } }; } + case UPDATE_IDP_SSO_URL: { + return { ...state, idp: { ...state.idp, ssoUrl: action.ssoUrl, isDirty: true } }; + } + case UPDATE_IDP_PUBLIC_KEY: { + return { ...state, idp: { ...state.idp, publicKey: action.publicKey, isDirty: true } }; + } case UPDATE_IDP_DIRTYSTATE: { return { ...state, idp: { ...state.idp, isDirty: action.dirtyState } }; } diff --git a/src/components/settings/SettingsSSOTab/hooks.js b/src/components/settings/SettingsSSOTab/hooks.js index c7535a83b0..f5360a2028 100644 --- a/src/components/settings/SettingsSSOTab/hooks.js +++ b/src/components/settings/SettingsSSOTab/hooks.js @@ -1,13 +1,15 @@ /* eslint-disable import/prefer-default-export */ -import { useEffect, useState, useContext } from 'react'; +import { + useEffect, useState, useContext, useCallback, +} from 'react'; import { logInfo } from '@edx/frontend-platform/logging'; import LmsApiService from '../../../data/services/LmsApiService'; import { SSOConfigContext } from './SSOConfigContext'; import { updateIdpMetadataURLAction, updateIdpEntryTypeAction, updateEntityIDAction, - updateIdpDirtyState, + updateIdpDirtyState, updateSsoUrlAction, updatePublicKeyAction, } from './data/actions'; -import { updateSamlProviderData } from './utils'; +import { updateSamlProviderData, deleteSamlProviderData } from './utils'; const useIdpState = () => { const { @@ -19,17 +21,34 @@ const useIdpState = () => { entityID, entryType, isDirty, + publicKey, + ssoUrl, }, } = ssoState; - const handleMetadataURLUpdate = event => dispatchSsoState(updateIdpMetadataURLAction(event.target.value)); - const handleMetadataEntryTypeUpdate = event => dispatchSsoState(updateIdpEntryTypeAction(event.target.value)); - const handleEntityIDUpdate = event => dispatchSsoState(updateEntityIDAction(event.target.value)); + const handleMetadataURLUpdate = useCallback((event) => { + dispatchSsoState(updateIdpMetadataURLAction(event.target.value)); + }, [dispatchSsoState]); + const handleMetadataEntryTypeUpdate = useCallback((event) => { + dispatchSsoState(updateIdpEntryTypeAction(event.target.value)); + }, [dispatchSsoState]); + const handleEntityIDUpdate = useCallback((event) => { + dispatchSsoState(updateEntityIDAction(event.target.value)); + }, [dispatchSsoState]); + const handleSsoUrlUpdate = useCallback((event) => { + dispatchSsoState(updateSsoUrlAction(event.target.value)); + }, [dispatchSsoState]); + const handlePublicKeyUpdate = useCallback((event) => { + dispatchSsoState(updatePublicKeyAction(event.target.value)); + }, [dispatchSsoState]); const createOrUpdateIdpRecord = async ({ enterpriseName, enterpriseSlug, enterpriseId, providerConfig = null, onSuccess, + existingIdpDataEntityId, + existingIdpDataId, + existingMetadataUrl, }) => { if (!isDirty && currentError === null) { dispatchSsoState(updateIdpDirtyState(false)); @@ -50,7 +69,13 @@ const useIdpState = () => { formData.append('slug', enterpriseSlug); formData.append('enabled', true); formData.append('enterprise_customer_uuid', enterpriseId); - formData.append('metadata_source', metadataURL); + if (entryType === 'url') { + formData.append('metadata_source', metadataURL); + } else { + // Direct entry of idp data won't include a metadata source so use a placeholder instead + const formattedName = enterpriseName.replace(/\s+/g, '-').toLowerCase(); + formData.append('metadata_source', `${formattedName}-placeholder.com/saml/fake.xml`); + } formData.append('entity_id', entityID); formData.append('skip_hinted_login_dialog', true); formData.append('skip_registration_form', true); @@ -75,14 +100,26 @@ const useIdpState = () => { // but we need to update this support the case when the correct sso config must be updated setProviderConfig(response.data); - // also get samlproviderdata updated from remote metadata url - const providerdataResponse = await updateSamlProviderData({ - enterpriseId, - metadataURL, - entityID, - }); - logInfo(providerdataResponse); + // If the user already has a provider data entry with a different entityID, remove it before + // creating a new one + if (existingIdpDataId && (existingIdpDataEntityId !== entityID)) { + await deleteSamlProviderData(existingIdpDataId, enterpriseId); + } + if ( + entryType === 'direct' || (existingMetadataUrl !== metadataURL || existingIdpDataEntityId !== entityID) + ) { + // also get samlproviderdata updated from remote metadata url + const providerdataResponse = await updateSamlProviderData({ + enterpriseId, + metadataURL, + entityID, + ssoUrl, + publicKey, + entryType, + }); + logInfo(providerdataResponse); + } setCurrentError(null); // then save samlproviderdata before running onSuccess callback onSuccess(); @@ -102,13 +139,46 @@ const useIdpState = () => { metadataURL, entryType, entityID, + ssoUrl, + publicKey, handleMetadataURLUpdate, handleMetadataEntryTypeUpdate, handleEntityIDUpdate, + handlePublicKeyUpdate, + handleSsoUrlUpdate, createOrUpdateIdpRecord, }; }; +const useExistingProviderData = (enterpriseUuid, refreshBool) => { + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + const [samlData, setSamlData] = useState({}); + + useEffect(() => { + if (enterpriseUuid) { + const fetchData = async () => { + const response = await LmsApiService.getProviderData(enterpriseUuid); + return response.data.results[0]; + }; + fetchData().then(data => { + setSamlData(data); + setLoading(false); + }).catch(err => { + setLoading(false); + if (err.customAttributes?.httpErrorStatus !== 404) { + // nothing found is okay for this fetcher. + setError(err); + } else { + setSamlData({}); + } + }); + } + }, [enterpriseUuid, refreshBool]); + + return [samlData, error, loading]; +}; + const useExistingSSOConfigs = (enterpriseUuid, refreshBool) => { const [ssoConfigs, setSsoConfigs] = useState([]); const [loading, setLoading] = useState(true); @@ -116,11 +186,11 @@ const useExistingSSOConfigs = (enterpriseUuid, refreshBool) => { useEffect(() => { if (enterpriseUuid) { - const fetchData = async () => { + const fetchConfig = async () => { const response = await LmsApiService.getProviderConfig(enterpriseUuid); return response.data.results; }; - fetchData().then(configs => { + fetchConfig().then(configs => { setSsoConfigs(configs); setLoading(false); }).catch(err => { @@ -134,10 +204,12 @@ const useExistingSSOConfigs = (enterpriseUuid, refreshBool) => { }); } }, [enterpriseUuid, refreshBool]); + return [ssoConfigs, error, loading]; }; export { useExistingSSOConfigs, + useExistingProviderData, useIdpState, }; diff --git a/src/components/settings/SettingsSSOTab/index.jsx b/src/components/settings/SettingsSSOTab/index.jsx index d0c4441206..fc205d1321 100644 --- a/src/components/settings/SettingsSSOTab/index.jsx +++ b/src/components/settings/SettingsSSOTab/index.jsx @@ -4,7 +4,7 @@ import Skeleton from 'react-loading-skeleton'; import { Alert, Hyperlink, Toast } from '@edx/paragon'; import { WarningFilled } from '@edx/paragon/icons'; import { HELP_CENTER_SAML_LINK } from '../data/constants'; -import { useExistingSSOConfigs } from './hooks'; +import { useExistingSSOConfigs, useExistingProviderData } from './hooks'; import ExistingSSOConfigs from './ExistingSSOConfigs'; import NewSSOConfigForm from './NewSSOConfigForm'; import { SSOConfigContext, SSOConfigContextProvider } from './SSOConfigContext'; @@ -16,6 +16,7 @@ const SettingsSSOTab = ({ enterpriseId }) => { setRefreshBool, } = useContext(SSOConfigContext); const [existingConfigs, error, isLoading] = useExistingSSOConfigs(enterpriseId, refreshBool); + const [existingProviderData, pdError, pdIsLoading] = useExistingProviderData(enterpriseId, refreshBool); return (
@@ -29,13 +30,14 @@ const SettingsSSOTab = ({ enterpriseId }) => { Help Center
- {!isLoading && ( + {(!isLoading || !pdIsLoading) && (
{/* providerConfig represents the currently selected config to edit/create, if there are existing configs but no providerConfig then we can safely render the listings page */} {existingConfigs?.length > 0 && (providerConfig === null) && ( { render the create/edit form */} {existingConfigs?.length > 0 && (providerConfig !== null) && } {error && ( - + An error occurred loading the SAML configs:

{error?.message}

 )} + {pdError && ( + + An error occurred loading the SAML data:

{pdError?.message}

+ + )} {infoMessage && ( setInfoMessage(null)}>{infoMessage})}
)} - {isLoading && } + {(isLoading || pdIsLoading) && }
); }; diff --git a/src/components/settings/SettingsSSOTab/steps/SSOConfigConfigureStep.jsx b/src/components/settings/SettingsSSOTab/steps/SSOConfigConfigureStep.jsx index cfd000e6bb..3bfb173262 100644 --- a/src/components/settings/SettingsSSOTab/steps/SSOConfigConfigureStep.jsx +++ b/src/components/settings/SettingsSSOTab/steps/SSOConfigConfigureStep.jsx @@ -271,7 +271,7 @@ SSOConfigConfigureStep.defaultProps = { lastName: '', max_session_length: undefined, emailAddress: '', - saml_configuration: '', + saml_configuration: -1, }, }; diff --git a/src/components/settings/SettingsSSOTab/steps/SSOConfigConfigureStep.test.jsx b/src/components/settings/SettingsSSOTab/steps/SSOConfigConfigureStep.test.jsx index 29b9015c7d..a8477328f1 100644 --- a/src/components/settings/SettingsSSOTab/steps/SSOConfigConfigureStep.test.jsx +++ b/src/components/settings/SettingsSSOTab/steps/SSOConfigConfigureStep.test.jsx @@ -21,7 +21,17 @@ describe('SSO Config Configure step', () => { render( - + , ); @@ -40,6 +50,12 @@ describe('SSO Config Configure step', () => { { { render( - + , ); diff --git a/src/components/settings/SettingsSSOTab/steps/SSOConfigConnectStep.jsx b/src/components/settings/SettingsSSOTab/steps/SSOConfigConnectStep.jsx index 2135b0dc9b..15ce64a4ef 100644 --- a/src/components/settings/SettingsSSOTab/steps/SSOConfigConnectStep.jsx +++ b/src/components/settings/SettingsSSOTab/steps/SSOConfigConnectStep.jsx @@ -14,9 +14,10 @@ const SSOConfigConnectStep = ({ // When we render this component, we need to re-fetch provider configs and update the store // so that we can correctly show latest state of providers // also, apply latest version of config to ssoState - const { ssoState: { providerConfig }, setProviderConfig } = useContext(SSOConfigContext); + const { ssoState, setProviderConfig } = useContext(SSOConfigContext); + const { providerConfig, refreshBool } = ssoState; + const [existingConfigs, error, isLoading] = useExistingSSOConfigs(enterpriseId, refreshBool); const { slug: idpSlug } = providerConfig; - const [existingConfigs, error, isLoading] = useExistingSSOConfigs(enterpriseId); useEffect(() => { if (isLoading) { return; } // don't want to do anything unless isLoading is done diff --git a/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.jsx b/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.jsx index 50fc441ea3..6737a64e5e 100644 --- a/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.jsx +++ b/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.jsx @@ -1,5 +1,10 @@ import { Form } from '@edx/paragon'; +import { useContext, useEffect } from 'react'; +import { connect } from 'react-redux'; +import PropTypes from 'prop-types'; import { useIdpState } from '../hooks'; +import { SSOConfigContext } from '../SSOConfigContext'; +import LmsApiService from '../../../../data/services/LmsApiService'; /** * This step will opt user to enter one of these options: @@ -7,13 +12,65 @@ import { useIdpState } from '../hooks'; * this is done in the backend. However, in this case, we don't yet know the entityId unless we read the file * therefore, we want the user to also enter their entityID */ -const SSOConfigIDPStep = () => { +const SSOConfigIDPStep = ({ + enterpriseId, + setExistingIdpDataEntityId, + setExistingIdpDataId, + setExistingMetadataUrl, +}) => { const { - metadataURL, entryType, entityID, + metadataURL, entryType, entityID, publicKey, ssoUrl, handleMetadataURLUpdate, handleMetadataEntryTypeUpdate, handleEntityIDUpdate, + handlePublicKeyUpdate, handleSsoUrlUpdate, } = useIdpState(); - const TITLE = 'First, select the method to provide your Identity Provider Metadata and fill out the corresponding fields. '; + const { setCurrentError, ssoState } = useContext(SSOConfigContext); + const { currentStep } = ssoState; + + const onIdpStep = currentStep === 'idp'; + + useEffect(() => { + setExistingMetadataUrl(metadataURL); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + useEffect(() => { + let isMounted = true; + const fetchData = async () => { + const response = await LmsApiService.getProviderData(enterpriseId); + return response.data.results[0]; + }; + fetchData().then(config => { + if (isMounted) { + handleEntityIDUpdate({ target: { value: config.entity_id } }); + handlePublicKeyUpdate({ target: { value: config.public_key } }); + handleSsoUrlUpdate({ target: { value: config.sso_url } }); + setExistingIdpDataEntityId(config.entity_id); + setExistingIdpDataId(config.id); + } + }).catch(err => { + if (err.customAttributes?.httpErrorStatus !== 404) { + setCurrentError(`: Failed to fetch provider data config. Server responded with: ${err}`); + } else { + handleEntityIDUpdate({ target: { value: undefined } }); + handlePublicKeyUpdate({ target: { value: undefined } }); + handleSsoUrlUpdate({ target: { value: undefined } }); + } + }); + return () => { isMounted = false; }; + }, [ + onIdpStep, + handleEntityIDUpdate, + handlePublicKeyUpdate, + handleSsoUrlUpdate, + setCurrentError, + enterpriseId, + setExistingIdpDataEntityId, + setExistingIdpDataId, + setExistingMetadataUrl, + ]); + + const TITLE = 'First, select the method to provide your Identity Provider Metadata and fill out the corresponding fields.'; return ( <> @@ -25,27 +82,73 @@ const SSOConfigIDPStep = () => { onChange={handleMetadataEntryTypeUpdate} value={entryType} > - Provide URL + + Provide URL + + +

+ You can find the URL in your Identiity Provider portal (on your IDP website?) +

+ + + Input data from a metadata file + + + - ); }; -export default SSOConfigIDPStep; +SSOConfigIDPStep.propTypes = { + enterpriseId: PropTypes.string.isRequired, + setExistingIdpDataEntityId: PropTypes.func.isRequired, + setExistingIdpDataId: PropTypes.func.isRequired, + setExistingMetadataUrl: PropTypes.func.isRequired, +}; + +const mapStateToProps = state => ({ + enterpriseId: state.portalConfiguration.enterpriseId, +}); + +export default connect(mapStateToProps)(SSOConfigIDPStep); diff --git a/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.test.jsx b/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.test.jsx index 605f5c38c3..6da6a5092d 100644 --- a/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.test.jsx +++ b/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.test.jsx @@ -1,4 +1,4 @@ -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; import { Provider } from 'react-redux'; @@ -7,7 +7,7 @@ import { SSOConfigContextProvider, SSO_INITIAL_STATE } from '../SSOConfigContext import { getMockStore, initialStore } from '../testutils'; describe('SSO Config IDP step, with no available providerConfig', () => { - test('renders page with metadata link', () => { + test('renders page with metadata link', async () => { const store = getMockStore({ ...initialStore }); const INITIAL_SSO_STATE = { ...SSO_INITIAL_STATE, @@ -18,12 +18,14 @@ describe('SSO Config IDP step, with no available providerConfig', () => { render( - + , ); - expect(screen.getByText('Provide URL')).toBeInTheDocument(); - expect(screen.getByText('Identity Provider Metadata URL')).toBeInTheDocument(); - expect(screen.getByText('Identity Provider EntityID')).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByText('Provide URL')).toBeInTheDocument(); + expect(screen.getByText('Identity Provider Metadata URL')).toBeInTheDocument(); + expect(screen.getByTestId('url-entry-entity-id')).toBeInTheDocument(); + }); }); }); diff --git a/src/components/settings/SettingsSSOTab/tests/ExistingSSOConfigs.test.jsx b/src/components/settings/SettingsSSOTab/tests/ExistingSSOConfigs.test.jsx index 02d394559e..0d464b8ead 100644 --- a/src/components/settings/SettingsSSOTab/tests/ExistingSSOConfigs.test.jsx +++ b/src/components/settings/SettingsSSOTab/tests/ExistingSSOConfigs.test.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { render, screen } from '@testing-library/react'; +import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import '@testing-library/jest-dom/extend-expect'; import configureMockStore from 'redux-mock-store'; @@ -7,7 +7,9 @@ import thunk from 'redux-thunk'; import { Provider } from 'react-redux'; import LmsApiService from '../../../../data/services/LmsApiService'; import ExistingSSOConfigs from '../ExistingSSOConfigs'; +import handleErrors from '../../utils'; +jest.mock('../../utils'); jest.mock('../../../../data/services/LmsApiService'); const enterpriseId = 'test-enterprise-id'; const mockSetRefreshBool = jest.fn(); @@ -52,8 +54,15 @@ const incompleteConfig = [ }, ]; +const providerData = { + id: 10, +}; + describe('', () => { - it('renders active config card', () => { + afterEach(() => { + jest.clearAllMocks(); + }); + it('renders active config card', async () => { render( ', () => { refreshBool setRefreshBool={mockSetRefreshBool} enterpriseId={enterpriseId} + providerData={providerData} /> , ); @@ -68,6 +78,7 @@ describe('', () => { expect(screen.getByText('Active')).toBeInTheDocument(); userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${activeConfig[0].id}`)); + expect(screen.getByText('Disable')).toBeInTheDocument(); expect(screen.getByText('Edit')).toBeInTheDocument(); @@ -75,7 +86,9 @@ describe('', () => { const data = new FormData(); data.append('enabled', false); data.append('enterprise_customer_uuid', enterpriseId); - expect(LmsApiService.updateProviderConfig).toHaveBeenCalledWith(data, activeConfig[0].id); + await waitFor(() => { + expect(LmsApiService.updateProviderConfig).toHaveBeenCalledWith(data, activeConfig[0].id); + }); }); it('renders inactive config card', () => { render( @@ -85,6 +98,7 @@ describe('', () => { refreshBool setRefreshBool={mockSetRefreshBool} enterpriseId={enterpriseId} + providerData={providerData} /> , ); @@ -101,7 +115,7 @@ describe('', () => { data.append('enterprise_customer_uuid', enterpriseId); expect(LmsApiService.updateProviderConfig).toHaveBeenCalledWith(data, inactiveConfig[0].id); }); - it('renders incomplete config card', () => { + it('renders incomplete config card', async () => { render( ', () => { refreshBool setRefreshBool={mockSetRefreshBool} enterpriseId={enterpriseId} + providerData={providerData} /> , ); expect(screen.getByText('bbq')).toBeInTheDocument(); expect(screen.getByText('Incomplete')).toBeInTheDocument(); userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`)); - expect(screen.getByText('Delete')).toBeInTheDocument(); - expect(screen.getByText('Edit')).toBeInTheDocument(); + await waitFor(() => { + expect(screen.getByText('Delete')).toBeInTheDocument(); + expect(screen.getByText('Edit')).toBeInTheDocument(); + }); }); it('renders multiple config cards', () => { render( @@ -126,6 +143,7 @@ describe('', () => { refreshBool setRefreshBool={mockSetRefreshBool} enterpriseId={enterpriseId} + providerData={providerData} /> , ); @@ -140,6 +158,7 @@ describe('', () => { refreshBool setRefreshBool={mockSetRefreshBool} enterpriseId={enterpriseId} + providerData={providerData} /> , ); @@ -147,4 +166,69 @@ describe('', () => { userEvent.click(screen.getByText('Delete')); expect(LmsApiService.deleteProviderConfig).toHaveBeenCalledWith(incompleteConfig[0].id, enterpriseId); }); + it('properly handles errors when deleting provider data', () => { + const mockDeleteProviderData = jest.spyOn(LmsApiService, 'deleteProviderData'); + mockDeleteProviderData.mockImplementation(() => { + throw new Error({ response: { data: 'foobar' } }); + }); + render( + + + , + ); + userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`)); + userEvent.click(screen.getByText('Delete')); + expect(handleErrors).toHaveBeenCalled(); + }); + it('properly handles errors when deleting provider configs', () => { + const mockDeleteProviderData = jest.spyOn(LmsApiService, 'deleteProviderConfig'); + mockDeleteProviderData.mockImplementation(() => { + throw new Error({ response: { data: 'foobar' } }); + }); + render( + + + , + ); + userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`)); + userEvent.click(screen.getByText('Delete')); + expect(handleErrors).toHaveBeenCalled(); + }); + it('properly displays error message when deleting provider configs', async () => { + const mockDeleteProviderData = jest.spyOn(LmsApiService, 'deleteProviderData'); + mockDeleteProviderData.mockImplementation(() => { + throw new Error({ response: { data: 'foobar' } }); + }); + handleErrors.mockResolvedValue('ayylmao'); + render( + + + , + ); + userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`)); + userEvent.click(screen.getByText('Delete')); + await waitFor(() => { + expect(screen.getByText( + 'We were unable to delete your configuration. Please try removing again or contact support for help.', + )).toBeInTheDocument(); + }, []); + }); }); diff --git a/src/components/settings/SettingsSSOTab/tests/NewSSOConfigForm.test.jsx b/src/components/settings/SettingsSSOTab/tests/NewSSOConfigForm.test.jsx index 27a7eb9674..53ac59ce9e 100644 --- a/src/components/settings/SettingsSSOTab/tests/NewSSOConfigForm.test.jsx +++ b/src/components/settings/SettingsSSOTab/tests/NewSSOConfigForm.test.jsx @@ -10,10 +10,32 @@ import { getMockStore, initialStore } from '../testutils'; import handleErrors from '../../utils'; jest.mock('../../utils'); +const entryType = 'direct'; +const metadataURL = ''; +const entityID = 'foobar'; +const publicKey = '123abc'; +const ssoUrl = 'https://foobar.com'; +const mockCreateOrUpdateIdpRecord = jest.fn(); +const mockHandlePublicKeyUpdate = jest.fn(); +const mockHandleSsoUrlUpdate = jest.fn(); +const mockHandleEntityIDUpdate = jest.fn(); +const mockHandleMetadataEntryTypeUpdate = jest.fn(); jest.mock('../hooks', () => { const originalModule = jest.requireActual('../hooks'); return { ...originalModule, + useIdpState: () => ({ + entryType, + metadataURL, + entityID, + publicKey, + ssoUrl, + createOrUpdateIdpRecord: mockCreateOrUpdateIdpRecord, + handleSsoUrlUpdate: mockHandleSsoUrlUpdate, + handlePublicKeyUpdate: mockHandlePublicKeyUpdate, + handleEntityIDUpdate: mockHandleEntityIDUpdate, + handleMetadataEntryTypeUpdate: mockHandleMetadataEntryTypeUpdate, + }), useExistingSSOConfigs: () => [[{ hehe: 'haha' }], null, true], }; }); @@ -21,7 +43,7 @@ jest.mock('../hooks', () => { const enterpriseId = 'an-id-1'; const store = getMockStore({ ...initialStore }); -const mockSetPRovderConfig = jest.fn(); +const mockSetProviderConfig = jest.fn(); const contextValue = { ...SSO_INITIAL_STATE, setCurrentError: jest.fn(), @@ -42,7 +64,7 @@ const contextValue = { id: 1337, }, }, - setProviderConfig: mockSetPRovderConfig, + setProviderConfig: mockSetProviderConfig, setRefreshBool: jest.fn(), }; @@ -71,7 +93,7 @@ describe('SAML Config Tab', () => { screen.queryByText('Loading SSO Configurations...'), ).toBeInTheDocument(); userEvent.click(screen.getByText('Cancel')); - await waitFor(() => expect(mockSetPRovderConfig).toHaveBeenCalledWith(null)); + await waitFor(() => expect(mockSetProviderConfig).toHaveBeenCalledWith(null)); }); test('canceling service provider step', async () => { contextValue.ssoState.currentStep = 'serviceprovider'; @@ -92,7 +114,7 @@ describe('SAML Config Tab', () => { screen.queryByText('I have added edX as a Service Provider in my SAML configuration'), ).toBeInTheDocument(); userEvent.click(screen.getByText('Cancel')); - expect(mockSetPRovderConfig).toHaveBeenCalledWith(null); + expect(mockSetProviderConfig).toHaveBeenCalledWith(null); }); test('error while configure step updating config from cancel button', async () => { // Setup @@ -137,7 +159,7 @@ describe('SAML Config Tab', () => { ).toBeInTheDocument(); userEvent.click(screen.getByText('Cancel')); expect(mockUpdateProviderConfig).not.toHaveBeenCalled(); - expect(mockSetPRovderConfig).toHaveBeenCalledWith(null); + expect(mockSetProviderConfig).toHaveBeenCalledWith(null); }); test('error while configure step updating config from next button', async () => { // Setup @@ -228,4 +250,88 @@ describe('SAML Config Tab', () => { userEvent.click(screen.getByText('Save')); await expect(mockUpdateProviderConfig).toHaveBeenCalled(); }); + test('configure step calls set provider config after updating', async () => { + // Setup + const mockUpdateProviderConfig = jest.spyOn(LmsApiService, 'updateProviderConfig'); + mockUpdateProviderConfig.mockResolvedValue({ data: { result: [{ woohoo: 'ayylmao!' }] } }); + contextValue.ssoState.currentStep = 'configure'; + render( + + + , + ); + expect( + screen.queryByText( + 'Connect to SAML identity provider for single sign-on, such as Okta or OneLogin to ' + + 'allow quick access to your organization\'s learning catalog.', + ), + ).toBeInTheDocument(); + userEvent.type(screen.getByText('Maximum Session Length (seconds)'), '2'); + userEvent.click(screen.getByText('Next')); + await waitFor(() => expect(mockSetProviderConfig).toHaveBeenCalled()); + }); + test('idp completed check for direct entry', async () => { + // Setup + contextValue.ssoState.currentStep = 'idp'; + render( + + + , + ); + await waitFor(() => { + expect( + screen.queryByText( + 'Connect to SAML identity provider for single sign-on, such as Okta or OneLogin to ' + + 'allow quick access to your organization\'s learning catalog.', + ), + ).toBeInTheDocument(); + expect(screen.getByText('Next')).not.toBeDisabled(); + }, []); + }); + test('idp step fetches existing idp data fields', async () => { + // Setup + const mockGetProviderData = jest.spyOn(LmsApiService, 'getProviderData'); + mockGetProviderData.mockResolvedValue( + { data: { results: [{ entity_id: 'ayylmao!', public_key: '123abc!', sso_url: 'https://ayylmao.com' }] } }, + ); + contextValue.ssoState.currentStep = 'idp'; + render( + + + , + ); + expect( + screen.queryByText( + 'Connect to SAML identity provider for single sign-on, such as Okta or OneLogin to ' + + 'allow quick access to your organization\'s learning catalog.', + ), + ).toBeInTheDocument(); + await waitFor(() => expect(mockHandleEntityIDUpdate).toHaveBeenCalledWith({ target: { value: 'ayylmao!' } })); + await waitFor(() => expect(mockHandlePublicKeyUpdate).toHaveBeenCalledWith({ target: { value: '123abc!' } })); + await waitFor(() => expect(mockHandleSsoUrlUpdate).toHaveBeenCalledWith({ target: { value: 'https://ayylmao.com' } })); + }); + test('idp step handles no existing idp data', async () => { + // Setup + const mockGetProviderData = jest.spyOn(LmsApiService, 'getProviderData'); + const providerDataNotFoundError = new Error('provider data not found'); + providerDataNotFoundError.customAttributes = { httpErrorStatus: 404 }; + mockGetProviderData.mockImplementation(() => { + throw providerDataNotFoundError; + }); + contextValue.ssoState.currentStep = 'idp'; + render( + + + , + ); + expect( + screen.queryByText( + 'Connect to SAML identity provider for single sign-on, such as Okta or OneLogin to ' + + 'allow quick access to your organization\'s learning catalog.', + ), + ).toBeInTheDocument(); + await waitFor(() => expect(mockHandleEntityIDUpdate).toHaveBeenCalledWith({ target: { value: undefined } })); + await waitFor(() => expect(mockHandlePublicKeyUpdate).toHaveBeenCalledWith({ target: { value: undefined } })); + await waitFor(() => expect(mockHandleSsoUrlUpdate).toHaveBeenCalledWith({ target: { value: undefined } })); + }); }); diff --git a/src/components/settings/SettingsSSOTab/tests/SSOConfigCard.test.jsx b/src/components/settings/SettingsSSOTab/tests/SSOConfigCard.test.jsx index b24074d71c..e679476c4d 100644 --- a/src/components/settings/SettingsSSOTab/tests/SSOConfigCard.test.jsx +++ b/src/components/settings/SettingsSSOTab/tests/SSOConfigCard.test.jsx @@ -16,11 +16,20 @@ describe('SSOConfigCard', () => { slug: 'slug-provider', }, }; - const config = { name: 'name-config', slug: 'slug', entity_id: 'entityId' }; + const config = { + name: 'name-config', + slug: 'slug', + entity_id: 'entityId', + id: 1, + }; render( - + , ); @@ -36,11 +45,20 @@ describe('SSOConfigCard', () => { }, connect: { ...SSO_INITIAL_STATE.connect, isSsoValid: true }, }; - const config = { name: 'name-config', slug: 'slug', entity_id: 'entityId' }; + const config = { + name: 'name-config', + slug: 'slug', + entity_id: 'entityId', + id: 1, + }; render( - + , ); diff --git a/src/components/settings/SettingsSSOTab/tests/utils.test.js b/src/components/settings/SettingsSSOTab/tests/utils.test.js index ec6c3267d4..e9466c174e 100644 --- a/src/components/settings/SettingsSSOTab/tests/utils.test.js +++ b/src/components/settings/SettingsSSOTab/tests/utils.test.js @@ -1,16 +1,56 @@ import LmsApiService from '../../../../data/services/LmsApiService'; -import { updateSamlProviderData } from '../utils'; +import { updateSamlProviderData, deleteSamlProviderData } from '../utils'; jest.mock('../../../../data/services/LmsApiService', () => ({ syncProviderData: jest.fn(), + deleteProviderData: jest.fn(), })); describe('update saml provider data', () => { - test('calls syncProviderData', async () => { + afterEach(() => { + jest.clearAllMocks(); + }); + test('calls syncProviderData with correct data under url entry', async () => { const enterpriseId = 'a-id'; const metadataURL = 'http://url'; const entityID = 'anId'; + const entryType = 'url'; + const formData = new FormData(); + formData.append('enterprise_customer_uuid', enterpriseId); + formData.append('entity_id', entityID); + formData.append('metadata_url', metadataURL); + LmsApiService.syncProviderData.mockResolvedValue({ lepard: 'def' }); + const value = await updateSamlProviderData({ + enterpriseId, metadataURL, entityID, entryType, + }); + expect(value).toStrictEqual({ lepard: 'def' }); + expect(LmsApiService.syncProviderData).toBeCalledWith(formData); + }); + test('calls syncProviderData with correct data under direct entry', async () => { + const enterpriseId = 'a-id'; + const entityID = 'anId'; + const ssoUrl = 'https://foobar.com'; + const publicKey = '123abc'; + const entryType = 'direct'; + const formData = new FormData(); + formData.append('enterprise_customer_uuid', enterpriseId); + formData.append('entity_id', entityID); + formData.append('sso_url', ssoUrl); + formData.append('public_key', publicKey); LmsApiService.syncProviderData.mockResolvedValue({ lepard: 'def' }); - const value = await updateSamlProviderData({ enterpriseId, metadataURL, entityID }); + const value = await updateSamlProviderData({ + enterpriseId, entityID, ssoUrl, publicKey, entryType, + }); + expect(value).toStrictEqual({ lepard: 'def' }); + expect(LmsApiService.syncProviderData).toBeCalledWith(formData); + }); +}); +describe('delete saml provider data', () => { + test('calls deleteProviderData', async () => { + const enterpriseId = 'a-id'; + const pdid = 1; + LmsApiService.deleteProviderData.mockResolvedValue({ lepard: 'def' }); + const value = await deleteSamlProviderData(pdid, enterpriseId); expect(value).toStrictEqual({ lepard: 'def' }); + expect(LmsApiService.deleteProviderData).toBeCalledWith(pdid, enterpriseId); }); }); diff --git a/src/components/settings/SettingsSSOTab/utils.js b/src/components/settings/SettingsSSOTab/utils.js index 357fe3426d..218a98fe18 100644 --- a/src/components/settings/SettingsSSOTab/utils.js +++ b/src/components/settings/SettingsSSOTab/utils.js @@ -1,12 +1,28 @@ /* eslint-disable import/prefer-default-export */ import LmsApiService from '../../../data/services/LmsApiService'; -async function updateSamlProviderData({ enterpriseId, metadataURL, entityID }) { +async function deleteSamlProviderData(pdid, enterpriseId) { + return LmsApiService.deleteProviderData(pdid, enterpriseId); +} + +async function updateSamlProviderData({ + enterpriseId, + metadataURL, + entityID, + ssoUrl, + publicKey, + entryType, +}) { const formData = new FormData(); formData.append('enterprise_customer_uuid', enterpriseId); - formData.append('metadata_url', metadataURL); formData.append('entity_id', entityID); + if (entryType === 'url') { + formData.append('metadata_url', metadataURL); + } else if (entryType === 'direct') { + formData.append('sso_url', ssoUrl); + formData.append('public_key', publicKey); + } return LmsApiService.syncProviderData(formData); } -export { updateSamlProviderData }; +export { updateSamlProviderData, deleteSamlProviderData }; From a441acd563607637cbd2fc590639e1ede645daca Mon Sep 17 00:00:00 2001 From: Alexander Sheehan Date: Fri, 3 Jun 2022 16:42:44 -0400 Subject: [PATCH 04/33] chore: redesigned connect step and rolling back idp data direct entry --- .../SettingsSSOTab/ExistingSSOConfigs.jsx | 12 +- .../SSOConfigConfiguredCard.jsx | 138 ++++++++------ .../settings/SettingsSSOTab/SSOStepper.jsx | 56 +++--- .../settings/SettingsSSOTab/data/actions.js | 12 -- .../settings/SettingsSSOTab/data/reducer.js | 9 +- .../settings/SettingsSSOTab/hooks.js | 74 +++----- .../settings/SettingsSSOTab/index.jsx | 9 +- .../steps/SSOConfigConnectStep.jsx | 15 +- .../SettingsSSOTab/steps/SSOConfigIDPStep.jsx | 179 ++++++------------ .../steps/SSOConfigIDPStep.test.jsx | 2 +- .../tests/ExistingSSOConfigs.test.jsx | 4 +- .../tests/NewSSOConfigForm.test.jsx | 42 +--- .../tests/SSOConfigCard.test.jsx | 35 ++-- .../SettingsSSOTab/tests/utils.test.js | 18 -- .../settings/SettingsSSOTab/utils.js | 10 +- src/components/settings/data/constants.js | 2 +- src/components/settings/settings.scss | 14 ++ 17 files changed, 264 insertions(+), 367 deletions(-) diff --git a/src/components/settings/SettingsSSOTab/ExistingSSOConfigs.jsx b/src/components/settings/SettingsSSOTab/ExistingSSOConfigs.jsx index 2145665e15..f5831a2ae5 100644 --- a/src/components/settings/SettingsSSOTab/ExistingSSOConfigs.jsx +++ b/src/components/settings/SettingsSSOTab/ExistingSSOConfigs.jsx @@ -144,7 +144,13 @@ const ExistingSSOConfigs = ({
{ - deleteProviderData(providerData.id); + // loop through provider data and delete each one + providerData.forEach((data) => { + // double check that we're deleting provider data for the current config + if (data.entity_id === config.entity_id) { + deleteProviderData(data.id); + } + }); deleteConfig(config.id); }} data-testid="dropdown-delete-item" @@ -179,9 +185,7 @@ const ExistingSSOConfigs = ({ ExistingSSOConfigs.propTypes = { configs: PropTypes.arrayOf(PropTypes.shape({})).isRequired, - providerData: PropTypes.shape({ - id: PropTypes.number, - }).isRequired, + providerData: PropTypes.arrayOf(PropTypes.shape({})).isRequired, refreshBool: PropTypes.bool.isRequired, setRefreshBool: PropTypes.func.isRequired, enterpriseId: PropTypes.string.isRequired, diff --git a/src/components/settings/SettingsSSOTab/SSOConfigConfiguredCard.jsx b/src/components/settings/SettingsSSOTab/SSOConfigConfiguredCard.jsx index 9933a5ecf8..174df7a443 100644 --- a/src/components/settings/SettingsSSOTab/SSOConfigConfiguredCard.jsx +++ b/src/components/settings/SettingsSSOTab/SSOConfigConfiguredCard.jsx @@ -1,8 +1,9 @@ import PropTypes from 'prop-types'; -import { Badge, Card, Hyperlink } from '@edx/paragon'; -import { useContext, useEffect, useState } from 'react'; -import Skeleton from 'react-loading-skeleton'; -import { logInfo } from '@edx/frontend-platform/logging'; +import { + Form, Icon, OverlayTrigger, Popover, Button, +} from '@edx/paragon'; +import { AddCircle, CheckCircle } from '@edx/paragon/icons'; +import React, { useContext, useEffect, useState } from 'react'; import { connect } from 'react-redux'; import { SSOConfigContext } from './SSOConfigContext'; import { updateConnectInProgress, updateCurrentStep } from './data/actions'; @@ -14,19 +15,16 @@ import LmsApiService from '../../../data/services/LmsApiService'; * This is the clickable card that is used to test the SSO config before we complete the config creation process. */ const SSOConfigConfiguredCard = ({ - config, testLink, enterpriseId, setConnectError, + config, testLink, enterpriseId, setConnectError, setShowValidatedText, showValidatedText, }) => { const { ssoState, dispatchSsoState, setProviderConfig, setCurrentError, setIsSsoValid, - setInfoMessage, setStartTime, + setInfoMessage, setStartTime, setRefreshBool, } = useContext(SSOConfigContext); const { - connect: { - startTime, - inProgress, - isSsoValid, - }, + connect: { startTime }, + refreshBool, } = ssoState; const setCurrentStep = step => dispatchSsoState(updateCurrentStep(step)); const [interval, setInterval] = useState(null); @@ -47,92 +45,106 @@ const SSOConfigConfiguredCard = ({ // at this point we want to take users to the listing page showing this config // setting providerConfig to null will do that! // because the SettingsSSOTab/index.jsx is looking for that value - setConnectError(true); setProviderConfig(null); setCurrentError(null); + setCurrentStep('idp'); + setRefreshBool(!refreshBool); } else if (performance.now() - startTime > SSO_CONFIG_POLLING_TIMEOUT) { + // We've run out of time setInterval(null); // disable the polling setIsSsoValid(false); setInfoMessage(null); dispatchSsoState(updateConnectInProgress(false)); setConnectError(true); - setCurrentStep('configure'); } } catch (error) { setCurrentError(error); setInterval(null); } }, interval); const initiateValidation = () => { - if (inProgress) { - // make no op in case click happens again during prior progress. - return; - } setStartTime(performance.now()); dispatchSsoState(updateConnectInProgress(true)); - window.open(testLink); setInterval(SSO_CONFIG_POLLING_INTERVAL); }; - // until isValid is false, keep checking server state (after about 3 minutes, reset and message customer) + useEffect(() => { - if (isSsoValid) { - setInterval(null); // just in case, disable the timer - dispatchSsoState(updateConnectInProgress(false)); - logInfo('SSO successfully validated'); + if (!config?.was_valid_at) { + initiateValidation(); } else { - // nothing to do right now - logInfo('Waiting for SSO valid to become true'); + setShowValidatedText(true); } - return function cleanup() { - setInterval(null); // so that when we unload, the timer is stopped - dispatchSsoState(updateConnectInProgress(false)); - }; - }, [isSsoValid, dispatchSsoState]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); - const handleTestClick = () => { - initiateValidation(); - return true; - }; return ( <> - {!inProgress && ( -

- Lastly, let us test your configuration. Click on a card below to connect to edX via your SSO. - (or visit this link in a separate browser/incognito window). -

- )} - {inProgress && (

- Please connect via SSO in the newly opened window - (or visit this link in a separate browser/incognito window). - Checking for successful SSO connection... + Lastly, let us test your configuration. Copy the link below and paste into a separate browser/incognito + window to connect to edX via your SSO.

+
+ + + + + Copied! + + + )} + > + + + +
+ { !showValidatedText && ( +

+ Once you've successfully logged in, + use this page to verify that your configuration is completed and validated. +

+ )} + { showValidatedText && ( +
+ +

+ You've successfully logged in with your SSO configuration, feel free to use the + link above in a private browser to test your configuration again or click Submit to finish. +

+
)} - {!inProgress ? ( - <> - - - - {!isSsoValid && configured}{' '} - {isSsoValid && completed}{' '} - - - - ) : Testing of SSO in progress...Please wait a bit or reload page at a different time.} ); }; SSOConfigConfiguredCard.propTypes = { config: PropTypes.shape({ - name: PropTypes.string.isRequired, - slug: PropTypes.string.isRequired, - id: PropTypes.number.isRequired, - entity_id: PropTypes.string.isRequired, - }).isRequired, + name: PropTypes.string, + slug: PropTypes.string, + id: PropTypes.number, + entity_id: PropTypes.string, + was_valid_at: PropTypes.string, + }), testLink: PropTypes.string.isRequired, enterpriseId: PropTypes.string.isRequired, setConnectError: PropTypes.func.isRequired, + setShowValidatedText: PropTypes.func.isRequired, + showValidatedText: PropTypes.bool.isRequired, +}; + +SSOConfigConfiguredCard.defaultProps = { + config: { + was_valid_at: '', name: '', slug: '', id: -1, entity_id: '', + }, }; const mapStateToProps = state => ({ diff --git a/src/components/settings/SettingsSSOTab/SSOStepper.jsx b/src/components/settings/SettingsSSOTab/SSOStepper.jsx index 55607155e4..a1403c56ba 100644 --- a/src/components/settings/SettingsSSOTab/SSOStepper.jsx +++ b/src/components/settings/SettingsSSOTab/SSOStepper.jsx @@ -8,7 +8,7 @@ import isURL from 'validator/lib/isURL'; import React, { useContext, useMemo, useState } from 'react'; import { connect } from 'react-redux'; import { updateCurrentStep } from './data/actions'; -import { useIdpState } from './hooks'; +import { useExistingProviderData, useIdpState } from './hooks'; import { SSOConfigContext } from './SSOConfigContext'; import SSOConfigConfigureStep from './steps/SSOConfigConfigureStep'; import SSOConfigIDPStep from './steps/SSOConfigIDPStep'; @@ -30,32 +30,19 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { const [connectError, setConnectError] = useState(false); const [showExitModal, setShowExitModal] = useState(false); const [formUpdated, setFormUpdated] = useState(false); - const [existingIdpDataEntityId, setExistingIdpDataEntityId] = useState(null); - const [existingIdpDataId, setExistingIdpDataId] = useState(null); - const [existingMetadataUrl, setExistingMetadataUrl] = useState(null); + const [existingProviderData] = useExistingProviderData(enterpriseId, refreshBool); + const [showValidatedText, setShowValidatedText] = useState(false); + const [idpNextButtonDisabled, setIdpNextButtonDisabled] = useState(false); const { - entryType, metadataURL, entityID, - publicKey, - ssoUrl, createOrUpdateIdpRecord, } = useIdpState(); - const isIdpStepComplete = useMemo(() => { - if (entryType === 'url') { - return (metadataURL && isURL(metadataURL)) && (entityID && !isEmpty(entityID)); - } - if (entryType === 'direct') { - return ( - (publicKey && !isEmpty(publicKey)) - && (entityID && !isEmpty(entityID)) - && (ssoUrl && isURL(ssoUrl)) - ); - } - return false; - }, [metadataURL, entityID, publicKey, ssoUrl, entryType]); + const isIdpStepComplete = useMemo(() => ( + (metadataURL && isURL(metadataURL)) && (entityID && !isEmpty(entityID)) + ), [metadataURL, entityID]); const handleCancel = () => { setCurrentStep('idp'); @@ -105,11 +92,7 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { - + @@ -132,7 +115,11 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { - + @@ -145,17 +132,19 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { )} + {showValidatedText && ( + + )}
diff --git a/src/components/settings/SettingsSSOTab/data/actions.js b/src/components/settings/SettingsSSOTab/data/actions.js index 023f0733d5..1cbe3287c7 100644 --- a/src/components/settings/SettingsSSOTab/data/actions.js +++ b/src/components/settings/SettingsSSOTab/data/actions.js @@ -1,8 +1,6 @@ export const UPDATE_IDP_METADATA_URL = 'update_idp_metadata_url'; export const UPDATE_IDP_ENTRY_TYPE = 'update_idp_entry_type'; export const UPDATE_IDP_ENTITYID = 'update_idp_entityid'; -export const UPDATE_IDP_SSO_URL = 'update_sso_url'; -export const UPDATE_IDP_PUBLIC_KEY = 'update_public_key'; export const UPDATE_IDP_DIRTYSTATE = 'update_idp_dirtystate'; export const UPDATE_CURRENT_STEP = 'update_current_step'; export const UPDATE_SP_CONFIGURED = 'update_sp_configured'; @@ -31,16 +29,6 @@ export const updateEntityIDAction = entityID => ({ entityID, }); -export const updateSsoUrlAction = ssoUrl => ({ - type: UPDATE_IDP_SSO_URL, - ssoUrl, -}); - -export const updatePublicKeyAction = publicKey => ({ - type: UPDATE_IDP_PUBLIC_KEY, - publicKey, -}); - export const updateIdpDirtyState = dirtyState => ({ type: UPDATE_IDP_DIRTYSTATE, dirtyState, diff --git a/src/components/settings/SettingsSSOTab/data/reducer.js b/src/components/settings/SettingsSSOTab/data/reducer.js index 4dd15a0891..986f915f3a 100644 --- a/src/components/settings/SettingsSSOTab/data/reducer.js +++ b/src/components/settings/SettingsSSOTab/data/reducer.js @@ -2,8 +2,7 @@ import { UPDATE_CURRENT_STEP, UPDATE_IDP_ENTRY_TYPE, UPDATE_IDP_METADATA_URL, UPDATE_SP_CONFIGURED, SET_PROVIDER_CONFIG, UPDATE_IDP_ENTITYID, UPDATE_CURRENT_ERROR, CLEAR_PROVIDER_CONFIG, UPDATE_IDP_DIRTYSTATE, UPDATE_CONNECT_IN_PROGRESS, UPDATE_CONNECT_IS_SSO_VALID, - UPDATE_INFO_MESSAGE, UPDATE_CONNECT_START_TIME, UPDATE_REFRESH_BOOL, UPDATE_IDP_SSO_URL, - UPDATE_IDP_PUBLIC_KEY, + UPDATE_INFO_MESSAGE, UPDATE_CONNECT_START_TIME, UPDATE_REFRESH_BOOL, } from './actions'; const SSOStateReducer = (state, action) => { @@ -23,12 +22,6 @@ const SSOStateReducer = (state, action) => { case UPDATE_IDP_ENTITYID: { return { ...state, idp: { ...state.idp, entityID: action.entityID, isDirty: true } }; } - case UPDATE_IDP_SSO_URL: { - return { ...state, idp: { ...state.idp, ssoUrl: action.ssoUrl, isDirty: true } }; - } - case UPDATE_IDP_PUBLIC_KEY: { - return { ...state, idp: { ...state.idp, publicKey: action.publicKey, isDirty: true } }; - } case UPDATE_IDP_DIRTYSTATE: { return { ...state, idp: { ...state.idp, isDirty: action.dirtyState } }; } diff --git a/src/components/settings/SettingsSSOTab/hooks.js b/src/components/settings/SettingsSSOTab/hooks.js index f5360a2028..197c64bd68 100644 --- a/src/components/settings/SettingsSSOTab/hooks.js +++ b/src/components/settings/SettingsSSOTab/hooks.js @@ -7,7 +7,7 @@ import LmsApiService from '../../../data/services/LmsApiService'; import { SSOConfigContext } from './SSOConfigContext'; import { updateIdpMetadataURLAction, updateIdpEntryTypeAction, updateEntityIDAction, - updateIdpDirtyState, updateSsoUrlAction, updatePublicKeyAction, + updateIdpDirtyState, } from './data/actions'; import { updateSamlProviderData, deleteSamlProviderData } from './utils'; @@ -34,21 +34,13 @@ const useIdpState = () => { const handleEntityIDUpdate = useCallback((event) => { dispatchSsoState(updateEntityIDAction(event.target.value)); }, [dispatchSsoState]); - const handleSsoUrlUpdate = useCallback((event) => { - dispatchSsoState(updateSsoUrlAction(event.target.value)); - }, [dispatchSsoState]); - const handlePublicKeyUpdate = useCallback((event) => { - dispatchSsoState(updatePublicKeyAction(event.target.value)); - }, [dispatchSsoState]); const createOrUpdateIdpRecord = async ({ enterpriseName, enterpriseSlug, enterpriseId, providerConfig = null, + existingProviderData, onSuccess, - existingIdpDataEntityId, - existingIdpDataId, - existingMetadataUrl, }) => { if (!isDirty && currentError === null) { dispatchSsoState(updateIdpDirtyState(false)); @@ -69,13 +61,7 @@ const useIdpState = () => { formData.append('slug', enterpriseSlug); formData.append('enabled', true); formData.append('enterprise_customer_uuid', enterpriseId); - if (entryType === 'url') { - formData.append('metadata_source', metadataURL); - } else { - // Direct entry of idp data won't include a metadata source so use a placeholder instead - const formattedName = enterpriseName.replace(/\s+/g, '-').toLowerCase(); - formData.append('metadata_source', `${formattedName}-placeholder.com/saml/fake.xml`); - } + formData.append('metadata_source', metadataURL); formData.append('entity_id', entityID); formData.append('skip_hinted_login_dialog', true); formData.append('skip_registration_form', true); @@ -102,24 +88,15 @@ const useIdpState = () => { // If the user already has a provider data entry with a different entityID, remove it before // creating a new one - if (existingIdpDataId && (existingIdpDataEntityId !== entityID)) { - await deleteSamlProviderData(existingIdpDataId, enterpriseId); - } + existingProviderData.forEach(async (idpData) => { + if (idpData.entity_id !== entityID) { + await deleteSamlProviderData(idpData.id, enterpriseId); + } + }); - if ( - entryType === 'direct' || (existingMetadataUrl !== metadataURL || existingIdpDataEntityId !== entityID) - ) { - // also get samlproviderdata updated from remote metadata url - const providerdataResponse = await updateSamlProviderData({ - enterpriseId, - metadataURL, - entityID, - ssoUrl, - publicKey, - entryType, - }); - logInfo(providerdataResponse); - } + // Make sure samlproviderdata is updated from remote metadata url + const providerdataResponse = await updateSamlProviderData({ enterpriseId, metadataURL, entityID }); + logInfo(providerdataResponse); setCurrentError(null); // then save samlproviderdata before running onSuccess callback onSuccess(); @@ -144,8 +121,6 @@ const useIdpState = () => { handleMetadataURLUpdate, handleMetadataEntryTypeUpdate, handleEntityIDUpdate, - handlePublicKeyUpdate, - handleSsoUrlUpdate, createOrUpdateIdpRecord, }; }; @@ -153,27 +128,34 @@ const useIdpState = () => { const useExistingProviderData = (enterpriseUuid, refreshBool) => { const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - const [samlData, setSamlData] = useState({}); + const [samlData, setSamlData] = useState([]); useEffect(() => { + let isMounted = true; if (enterpriseUuid) { const fetchData = async () => { const response = await LmsApiService.getProviderData(enterpriseUuid); - return response.data.results[0]; + // SAML provider data is returned as a list as there can be multiple per configuration + return response.data.results; }; fetchData().then(data => { - setSamlData(data); - setLoading(false); + if (isMounted) { + setSamlData(data); + setLoading(false); + } }).catch(err => { - setLoading(false); - if (err.customAttributes?.httpErrorStatus !== 404) { - // nothing found is okay for this fetcher. - setError(err); - } else { - setSamlData({}); + if (isMounted) { + setLoading(false); + if (err.customAttributes?.httpErrorStatus !== 404) { + // nothing found is okay for this fetcher. + setError(err); + } else { + setSamlData([]); + } } }); } + return () => { isMounted = false; }; }, [enterpriseUuid, refreshBool]); return [samlData, error, loading]; diff --git a/src/components/settings/SettingsSSOTab/index.jsx b/src/components/settings/SettingsSSOTab/index.jsx index fc205d1321..cfcca01e5f 100644 --- a/src/components/settings/SettingsSSOTab/index.jsx +++ b/src/components/settings/SettingsSSOTab/index.jsx @@ -58,7 +58,14 @@ const SettingsSSOTab = ({ enterpriseId }) => { An error occurred loading the SAML data:

{pdError?.message}

)} - {infoMessage && ( setInfoMessage(null)}>{infoMessage})} + {infoMessage && ( + setInfoMessage(null)} + show={infoMessage.length > 0} + > + {infoMessage} + + )} )} {(isLoading || pdIsLoading) && } diff --git a/src/components/settings/SettingsSSOTab/steps/SSOConfigConnectStep.jsx b/src/components/settings/SettingsSSOTab/steps/SSOConfigConnectStep.jsx index 15ce64a4ef..fa97f4b0a5 100644 --- a/src/components/settings/SettingsSSOTab/steps/SSOConfigConnectStep.jsx +++ b/src/components/settings/SettingsSSOTab/steps/SSOConfigConnectStep.jsx @@ -9,7 +9,7 @@ import { SSOConfigContext } from '../SSOConfigContext'; import { createSAMLURLs } from '../../../SamlProviderConfiguration/utils'; const SSOConfigConnectStep = ({ - enterpriseId, enterpriseSlug, learnerPortalEnabled, setConnectError, + enterpriseId, enterpriseSlug, learnerPortalEnabled, setConnectError, setShowValidatedText, showValidatedText, }) => { // When we render this component, we need to re-fetch provider configs and update the store // so that we can correctly show latest state of providers @@ -17,12 +17,15 @@ const SSOConfigConnectStep = ({ const { ssoState, setProviderConfig } = useContext(SSOConfigContext); const { providerConfig, refreshBool } = ssoState; const [existingConfigs, error, isLoading] = useExistingSSOConfigs(enterpriseId, refreshBool); - const { slug: idpSlug } = providerConfig; + const idpSlug = ssoState.providerConfig?.slug; useEffect(() => { if (isLoading) { return; } // don't want to do anything unless isLoading is done - const updatedProviderConfig = existingConfigs.filter(config => config.id === providerConfig.id).shift(); - setProviderConfig(updatedProviderConfig); + if (existingConfigs.length > 0 && providerConfig) { + const updatedProviderConfig = existingConfigs.filter(config => config.id === providerConfig.id) + .shift(); + setProviderConfig(updatedProviderConfig); + } }, [isLoading, existingConfigs, setProviderConfig, providerConfig]); const configuration = getConfig(); @@ -43,6 +46,8 @@ const SSOConfigConnectStep = ({ config={providerConfig} testLink={testLink} setConnectError={setConnectError} + setShowValidatedText={setShowValidatedText} + showValidatedText={showValidatedText} /> @@ -63,6 +68,8 @@ SSOConfigConnectStep.propTypes = { enterpriseSlug: PropTypes.string.isRequired, learnerPortalEnabled: PropTypes.bool.isRequired, setConnectError: PropTypes.func.isRequired, + setShowValidatedText: PropTypes.func.isRequired, + showValidatedText: PropTypes.bool.isRequired, }; const mapStateToProps = state => ({ diff --git a/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.jsx b/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.jsx index 6737a64e5e..319809098b 100644 --- a/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.jsx +++ b/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.jsx @@ -1,10 +1,9 @@ import { Form } from '@edx/paragon'; -import { useContext, useEffect } from 'react'; +import React, { useContext } from 'react'; import { connect } from 'react-redux'; import PropTypes from 'prop-types'; -import { useIdpState } from '../hooks'; +import { useIdpState, useExistingProviderData } from '../hooks'; import { SSOConfigContext } from '../SSOConfigContext'; -import LmsApiService from '../../../../data/services/LmsApiService'; /** * This step will opt user to enter one of these options: @@ -12,63 +11,12 @@ import LmsApiService from '../../../../data/services/LmsApiService'; * this is done in the backend. However, in this case, we don't yet know the entityId unless we read the file * therefore, we want the user to also enter their entityID */ -const SSOConfigIDPStep = ({ - enterpriseId, - setExistingIdpDataEntityId, - setExistingIdpDataId, - setExistingMetadataUrl, -}) => { +const SSOConfigIDPStep = ({ enterpriseId }) => { const { - metadataURL, entryType, entityID, publicKey, ssoUrl, - handleMetadataURLUpdate, handleMetadataEntryTypeUpdate, handleEntityIDUpdate, - handlePublicKeyUpdate, handleSsoUrlUpdate, + metadataURL, entityID, handleMetadataURLUpdate, handleEntityIDUpdate, } = useIdpState(); - - const { setCurrentError, ssoState } = useContext(SSOConfigContext); - const { currentStep } = ssoState; - - const onIdpStep = currentStep === 'idp'; - - useEffect(() => { - setExistingMetadataUrl(metadataURL); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); - - useEffect(() => { - let isMounted = true; - const fetchData = async () => { - const response = await LmsApiService.getProviderData(enterpriseId); - return response.data.results[0]; - }; - fetchData().then(config => { - if (isMounted) { - handleEntityIDUpdate({ target: { value: config.entity_id } }); - handlePublicKeyUpdate({ target: { value: config.public_key } }); - handleSsoUrlUpdate({ target: { value: config.sso_url } }); - setExistingIdpDataEntityId(config.entity_id); - setExistingIdpDataId(config.id); - } - }).catch(err => { - if (err.customAttributes?.httpErrorStatus !== 404) { - setCurrentError(`: Failed to fetch provider data config. Server responded with: ${err}`); - } else { - handleEntityIDUpdate({ target: { value: undefined } }); - handlePublicKeyUpdate({ target: { value: undefined } }); - handleSsoUrlUpdate({ target: { value: undefined } }); - } - }); - return () => { isMounted = false; }; - }, [ - onIdpStep, - handleEntityIDUpdate, - handlePublicKeyUpdate, - handleSsoUrlUpdate, - setCurrentError, - enterpriseId, - setExistingIdpDataEntityId, - setExistingIdpDataId, - setExistingMetadataUrl, - ]); + const { refreshBool } = useContext(SSOConfigContext); + const [existingProviderData] = useExistingProviderData(enterpriseId, refreshBool); const TITLE = 'First, select the method to provide your Identity Provider Metadata and fill out the corresponding fields.'; @@ -77,63 +25,61 @@ const SSOConfigIDPStep = ({ {TITLE}
- - - Provide URL - - - -

- You can find the URL in your Identiity Provider portal (on your IDP website?) -

- - - Input data from a metadata file - - - - - + + Metadata Source Information: + + + +

+ You can find the URL in your Identiity Provider portal or on your IDP website. +

+ { existingProviderData.length > 0 && ( + <> + + Existing certificate data + + + + { existingProviderData.map((dataConfig, index) => ( + + + + ))} + + )}
@@ -142,9 +88,6 @@ const SSOConfigIDPStep = ({ SSOConfigIDPStep.propTypes = { enterpriseId: PropTypes.string.isRequired, - setExistingIdpDataEntityId: PropTypes.func.isRequired, - setExistingIdpDataId: PropTypes.func.isRequired, - setExistingMetadataUrl: PropTypes.func.isRequired, }; const mapStateToProps = state => ({ diff --git a/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.test.jsx b/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.test.jsx index 6da6a5092d..6d68aa7cec 100644 --- a/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.test.jsx +++ b/src/components/settings/SettingsSSOTab/steps/SSOConfigIDPStep.test.jsx @@ -23,7 +23,7 @@ describe('SSO Config IDP step, with no available providerConfig', () => { , ); await waitFor(() => { - expect(screen.getByText('Provide URL')).toBeInTheDocument(); + expect(screen.getByText('Metadata Source Information:')).toBeInTheDocument(); expect(screen.getByText('Identity Provider Metadata URL')).toBeInTheDocument(); expect(screen.getByTestId('url-entry-entity-id')).toBeInTheDocument(); }); diff --git a/src/components/settings/SettingsSSOTab/tests/ExistingSSOConfigs.test.jsx b/src/components/settings/SettingsSSOTab/tests/ExistingSSOConfigs.test.jsx index 0d464b8ead..b2a3a31a7b 100644 --- a/src/components/settings/SettingsSSOTab/tests/ExistingSSOConfigs.test.jsx +++ b/src/components/settings/SettingsSSOTab/tests/ExistingSSOConfigs.test.jsx @@ -54,9 +54,9 @@ const incompleteConfig = [ }, ]; -const providerData = { +const providerData = [{ id: 10, -}; +}]; describe('', () => { afterEach(() => { diff --git a/src/components/settings/SettingsSSOTab/tests/NewSSOConfigForm.test.jsx b/src/components/settings/SettingsSSOTab/tests/NewSSOConfigForm.test.jsx index 53ac59ce9e..f04b11958c 100644 --- a/src/components/settings/SettingsSSOTab/tests/NewSSOConfigForm.test.jsx +++ b/src/components/settings/SettingsSSOTab/tests/NewSSOConfigForm.test.jsx @@ -11,13 +11,11 @@ import handleErrors from '../../utils'; jest.mock('../../utils'); const entryType = 'direct'; -const metadataURL = ''; +const metadataURL = 'https://foobar.com'; const entityID = 'foobar'; -const publicKey = '123abc'; +const publicKey = 'abc123'; const ssoUrl = 'https://foobar.com'; const mockCreateOrUpdateIdpRecord = jest.fn(); -const mockHandlePublicKeyUpdate = jest.fn(); -const mockHandleSsoUrlUpdate = jest.fn(); const mockHandleEntityIDUpdate = jest.fn(); const mockHandleMetadataEntryTypeUpdate = jest.fn(); jest.mock('../hooks', () => { @@ -31,8 +29,6 @@ jest.mock('../hooks', () => { publicKey, ssoUrl, createOrUpdateIdpRecord: mockCreateOrUpdateIdpRecord, - handleSsoUrlUpdate: mockHandleSsoUrlUpdate, - handlePublicKeyUpdate: mockHandlePublicKeyUpdate, handleEntityIDUpdate: mockHandleEntityIDUpdate, handleMetadataEntryTypeUpdate: mockHandleMetadataEntryTypeUpdate, }), @@ -270,7 +266,7 @@ describe('SAML Config Tab', () => { userEvent.click(screen.getByText('Next')); await waitFor(() => expect(mockSetProviderConfig).toHaveBeenCalled()); }); - test('idp completed check for direct entry', async () => { + test('idp completed check for url entry', async () => { // Setup contextValue.ssoState.currentStep = 'idp'; render( @@ -288,7 +284,7 @@ describe('SAML Config Tab', () => { expect(screen.getByText('Next')).not.toBeDisabled(); }, []); }); - test('idp step fetches existing idp data fields', async () => { + test('idp step fetches and displays existing idp data fields', async () => { // Setup const mockGetProviderData = jest.spyOn(LmsApiService, 'getProviderData'); mockGetProviderData.mockResolvedValue( @@ -306,32 +302,8 @@ describe('SAML Config Tab', () => { + 'allow quick access to your organization\'s learning catalog.', ), ).toBeInTheDocument(); - await waitFor(() => expect(mockHandleEntityIDUpdate).toHaveBeenCalledWith({ target: { value: 'ayylmao!' } })); - await waitFor(() => expect(mockHandlePublicKeyUpdate).toHaveBeenCalledWith({ target: { value: '123abc!' } })); - await waitFor(() => expect(mockHandleSsoUrlUpdate).toHaveBeenCalledWith({ target: { value: 'https://ayylmao.com' } })); - }); - test('idp step handles no existing idp data', async () => { - // Setup - const mockGetProviderData = jest.spyOn(LmsApiService, 'getProviderData'); - const providerDataNotFoundError = new Error('provider data not found'); - providerDataNotFoundError.customAttributes = { httpErrorStatus: 404 }; - mockGetProviderData.mockImplementation(() => { - throw providerDataNotFoundError; - }); - contextValue.ssoState.currentStep = 'idp'; - render( - - - , - ); - expect( - screen.queryByText( - 'Connect to SAML identity provider for single sign-on, such as Okta or OneLogin to ' - + 'allow quick access to your organization\'s learning catalog.', - ), - ).toBeInTheDocument(); - await waitFor(() => expect(mockHandleEntityIDUpdate).toHaveBeenCalledWith({ target: { value: undefined } })); - await waitFor(() => expect(mockHandlePublicKeyUpdate).toHaveBeenCalledWith({ target: { value: undefined } })); - await waitFor(() => expect(mockHandleSsoUrlUpdate).toHaveBeenCalledWith({ target: { value: undefined } })); + await waitFor(() => expect(screen.getByText('123abc!')).toBeInTheDocument()); + await waitFor(() => expect(screen.getByDisplayValue('ayylmao!')).toBeInTheDocument()); + await waitFor(() => expect(screen.getByDisplayValue('https://ayylmao.com')).toBeInTheDocument()); }); }); diff --git a/src/components/settings/SettingsSSOTab/tests/SSOConfigCard.test.jsx b/src/components/settings/SettingsSSOTab/tests/SSOConfigCard.test.jsx index e679476c4d..f0a552138a 100644 --- a/src/components/settings/SettingsSSOTab/tests/SSOConfigCard.test.jsx +++ b/src/components/settings/SettingsSSOTab/tests/SSOConfigCard.test.jsx @@ -9,7 +9,7 @@ import SSOConfigConfiguredCard from '../SSOConfigConfiguredCard'; describe('SSOConfigCard', () => { const store = getMockStore({ ...initialStore }); - test('render default card state when isSsoValid is false', () => { + test('render landing text when config isnt validated', () => { const INITIAL_SSO_STATE = { ...SSO_INITIAL_STATE, providerConfig: { @@ -29,15 +29,28 @@ describe('SSOConfigCard', () => { config={config} testLink="http://test" setConnectError={jest.fn()} + setShowValidatedText={jest.fn()} + showValidatedText={false} /> , ); - expect(screen.getByText('name-config')).toBeInTheDocument(); - expect(screen.getByText('configured')).toBeInTheDocument(); - expect(screen.queryByText('completed')).not.toBeInTheDocument(); + expect(screen.getByText( + 'Once you\'ve successfully logged in, use this page to verify that your configuration is completed and validated.', + )).toBeInTheDocument(); + expect(screen.getByDisplayValue('http://test')).toBeInTheDocument(); }); - test('render completed card state when inProgress is false and isSsoValid true', () => { + test('setting completed state when wav valid at is set on render', () => { + const configName = 'name-config'; + const configEntityId = 'entityId'; + const config = { + name: configName, + slug: 'slug', + entity_id: configEntityId, + id: 1, + was_valid_at: '10/10/2020', + }; + const mockSetShowValidatedText = jest.fn(); const INITIAL_SSO_STATE = { ...SSO_INITIAL_STATE, providerConfig: { @@ -45,12 +58,6 @@ describe('SSOConfigCard', () => { }, connect: { ...SSO_INITIAL_STATE.connect, isSsoValid: true }, }; - const config = { - name: 'name-config', - slug: 'slug', - entity_id: 'entityId', - id: 1, - }; render( @@ -58,12 +65,12 @@ describe('SSOConfigCard', () => { config={config} testLink="http://test" setConnectError={jest.fn()} + setShowValidatedText={mockSetShowValidatedText} + showValidatedText /> , ); - expect(screen.getByText('name-config')).toBeInTheDocument(); - expect(screen.queryByText('configured')).not.toBeInTheDocument(); - expect(screen.queryByText('completed')).toBeInTheDocument(); + expect(mockSetShowValidatedText).toHaveBeenCalledWith(true); }); }); diff --git a/src/components/settings/SettingsSSOTab/tests/utils.test.js b/src/components/settings/SettingsSSOTab/tests/utils.test.js index e9466c174e..fe3c72d45a 100644 --- a/src/components/settings/SettingsSSOTab/tests/utils.test.js +++ b/src/components/settings/SettingsSSOTab/tests/utils.test.js @@ -25,24 +25,6 @@ describe('update saml provider data', () => { expect(value).toStrictEqual({ lepard: 'def' }); expect(LmsApiService.syncProviderData).toBeCalledWith(formData); }); - test('calls syncProviderData with correct data under direct entry', async () => { - const enterpriseId = 'a-id'; - const entityID = 'anId'; - const ssoUrl = 'https://foobar.com'; - const publicKey = '123abc'; - const entryType = 'direct'; - const formData = new FormData(); - formData.append('enterprise_customer_uuid', enterpriseId); - formData.append('entity_id', entityID); - formData.append('sso_url', ssoUrl); - formData.append('public_key', publicKey); - LmsApiService.syncProviderData.mockResolvedValue({ lepard: 'def' }); - const value = await updateSamlProviderData({ - enterpriseId, entityID, ssoUrl, publicKey, entryType, - }); - expect(value).toStrictEqual({ lepard: 'def' }); - expect(LmsApiService.syncProviderData).toBeCalledWith(formData); - }); }); describe('delete saml provider data', () => { test('calls deleteProviderData', async () => { diff --git a/src/components/settings/SettingsSSOTab/utils.js b/src/components/settings/SettingsSSOTab/utils.js index 218a98fe18..a18368ab0b 100644 --- a/src/components/settings/SettingsSSOTab/utils.js +++ b/src/components/settings/SettingsSSOTab/utils.js @@ -9,19 +9,11 @@ async function updateSamlProviderData({ enterpriseId, metadataURL, entityID, - ssoUrl, - publicKey, - entryType, }) { const formData = new FormData(); formData.append('enterprise_customer_uuid', enterpriseId); formData.append('entity_id', entityID); - if (entryType === 'url') { - formData.append('metadata_url', metadataURL); - } else if (entryType === 'direct') { - formData.append('sso_url', ssoUrl); - formData.append('public_key', publicKey); - } + formData.append('metadata_url', metadataURL); return LmsApiService.syncProviderData(formData); } diff --git a/src/components/settings/data/constants.js b/src/components/settings/data/constants.js index 64d810761b..07b1d3f81a 100644 --- a/src/components/settings/data/constants.js +++ b/src/components/settings/data/constants.js @@ -69,7 +69,7 @@ export const BLACKBOARD_OAUTH_REDIRECT_URL = `${configuration.LMS_BASE_URL}/blac export const CANVAS_OAUTH_REDIRECT_URL = `${configuration.LMS_BASE_URL}/canvas/oauth-complete`; export const LMS_CONFIG_OAUTH_POLLING_TIMEOUT = 60000; export const LMS_CONFIG_OAUTH_POLLING_INTERVAL = 1000; -export const SSO_CONFIG_POLLING_TIMEOUT = 120000; +export const SSO_CONFIG_POLLING_TIMEOUT = 240000; export const SSO_CONFIG_POLLING_INTERVAL = 1000; export const SUBSIDY_TYPE_LABELS = { diff --git a/src/components/settings/settings.scss b/src/components/settings/settings.scss index 89aa661761..dacb29c8de 100644 --- a/src/components/settings/settings.scss +++ b/src/components/settings/settings.scss @@ -70,3 +70,17 @@ .sso-create-form-control { width: 100%; } + +.sso-connect-step-copy-form > .form-control { + border-top-right-radius: 0 !important; + border-bottom-right-radius: 0 !important; +} + +.sso-connect-step-copy-button { + border-top-left-radius: 0 !important; + border-bottom-left-radius: 0 !important; +} + +.sso-connect-success-icon { + color: green; +} From 0ab3c242982f7e2565976791258bf225d98c89d9 Mon Sep 17 00:00:00 2001 From: Abdullah Waheed Date: Thu, 9 Jun 2022 14:56:19 +0500 Subject: [PATCH 05/33] refactor: updated StatusAlert deprecated component to Alert and made changes accordingly --- .../Admin/__snapshots__/Admin.test.jsx.snap | 46 +++++----- .../CodeSearchResults.test.jsx.snap | 84 +++++++++++-------- .../CompletedLearnersTable.test.jsx.snap | 38 +++++---- ...rnersForInactiveCoursesTable.test.jsx.snap | 38 +++++---- .../EnrolledLearnersTable.test.jsx.snap | 38 +++++---- .../EnrollmentsTable.test.jsx.snap | 38 +++++---- .../LearnerActivityTable.test.jsx.snap | 38 +++++---- .../RegisteredLearnersTable.test.jsx.snap | 38 +++++---- src/components/StatusAlert/index.jsx | 49 +++++------ .../CouponDetails/CouponDetails.test.jsx | 10 +-- 10 files changed, 245 insertions(+), 172 deletions(-) diff --git a/src/components/Admin/__snapshots__/Admin.test.jsx.snap b/src/components/Admin/__snapshots__/Admin.test.jsx.snap index 4caf8e4944..03992c7078 100644 --- a/src/components/Admin/__snapshots__/Admin.test.jsx.snap +++ b/src/components/Admin/__snapshots__/Admin.test.jsx.snap @@ -6118,34 +6118,42 @@ exports[` renders correctly with error state 1`] = ` className="col" > diff --git a/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap b/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap index eb1c9a664b..7ba2fe5125 100644 --- a/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap +++ b/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap @@ -55,29 +55,37 @@ exports[` basic rendering should render empty table data 1` diff --git a/src/components/CompletedLearnersTable/__snapshots__/CompletedLearnersTable.test.jsx.snap b/src/components/CompletedLearnersTable/__snapshots__/CompletedLearnersTable.test.jsx.snap index 54d25a5ba8..f261cdda4e 100644 --- a/src/components/CompletedLearnersTable/__snapshots__/CompletedLearnersTable.test.jsx.snap +++ b/src/components/CompletedLearnersTable/__snapshots__/CompletedLearnersTable.test.jsx.snap @@ -2,29 +2,37 @@ exports[`CompletedLearnersTable renders empty state correctly 1`] = ` diff --git a/src/components/EnrollmentsTable/__snapshots__/EnrollmentsTable.test.jsx.snap b/src/components/EnrollmentsTable/__snapshots__/EnrollmentsTable.test.jsx.snap index 4cea570f01..290ee70e95 100644 --- a/src/components/EnrollmentsTable/__snapshots__/EnrollmentsTable.test.jsx.snap +++ b/src/components/EnrollmentsTable/__snapshots__/EnrollmentsTable.test.jsx.snap @@ -3,21 +3,30 @@ exports[`EnrollmentsTable renders empty state correctly 1`] = `
-
- -
+ +
-
-
- There are no results. -
-
+ There are no results.
diff --git a/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap b/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap index 18c28847d5..c0fafc956e 100644 --- a/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap +++ b/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap @@ -422,21 +422,30 @@ exports[`LearnerActivityTable renders active learners table correctly 1`] = ` exports[`LearnerActivityTable renders empty state correctly 1`] = `
-
- -
+ +
-
-
- There are no results. -
-
+ There are no results.
diff --git a/src/components/LmsConfigurations/BlackboardIntegrationConfigForm.jsx b/src/components/LmsConfigurations/BlackboardIntegrationConfigForm.jsx index a756370a8f..d19abb7790 100644 --- a/src/components/LmsConfigurations/BlackboardIntegrationConfigForm.jsx +++ b/src/components/LmsConfigurations/BlackboardIntegrationConfigForm.jsx @@ -7,10 +7,12 @@ import { StatefulButton, Icon, Hyperlink, + Alert, } from '@edx/paragon'; +import { Error } from '@edx/paragon/icons'; + import { snakeCaseFormData } from '../../utils'; import LmsApiService from '../../data/services/LmsApiService'; -import StatusAlert from '../StatusAlert'; import SUBMIT_STATES from '../../data/constants/formSubmissions'; import { handleErrors, validateLmsConfigForm } from './common'; @@ -105,12 +107,15 @@ class BlackboardIntegrationConfigForm extends React.Component { if (error) { errorAlert = (
- + + + Unable to submit config form: + +

{error}

+
); } diff --git a/src/components/LmsConfigurations/CanvasIntegrationConfigForm.jsx b/src/components/LmsConfigurations/CanvasIntegrationConfigForm.jsx index 23973bed6f..944e549d34 100644 --- a/src/components/LmsConfigurations/CanvasIntegrationConfigForm.jsx +++ b/src/components/LmsConfigurations/CanvasIntegrationConfigForm.jsx @@ -2,10 +2,11 @@ import React from 'react'; import PropTypes from 'prop-types'; import isEmpty from 'lodash/isEmpty'; import { - ValidationFormGroup, Input, StatefulButton, Icon, + ValidationFormGroup, Input, StatefulButton, Icon, Alert, } from '@edx/paragon'; +import { Error } from '@edx/paragon/icons'; + import { snakeCaseFormData } from '../../utils'; -import StatusAlert from '../StatusAlert'; import LmsApiService from '../../data/services/LmsApiService'; import SUBMIT_STATES from '../../data/constants/formSubmissions'; import { handleErrors, validateLmsConfigForm } from './common'; @@ -96,12 +97,15 @@ class CanvasIntegrationConfigForm extends React.Component { if (error) { errorAlert = (
- + + + Unable to submit config form: + +

{error}

+
); } diff --git a/src/components/LmsConfigurations/CornerstoneIntegrationConfigForm.jsx b/src/components/LmsConfigurations/CornerstoneIntegrationConfigForm.jsx index 3c5729d369..2c4ffc45fa 100644 --- a/src/components/LmsConfigurations/CornerstoneIntegrationConfigForm.jsx +++ b/src/components/LmsConfigurations/CornerstoneIntegrationConfigForm.jsx @@ -2,11 +2,12 @@ import React, { useState } from 'react'; import PropTypes from 'prop-types'; import isEmpty from 'lodash/isEmpty'; import { - ValidationFormGroup, Input, StatefulButton, Icon, + ValidationFormGroup, Input, StatefulButton, Icon, Alert, } from '@edx/paragon'; +import { Error } from '@edx/paragon/icons'; + import { snakeCaseFormData } from '../../utils'; import LmsApiService from '../../data/services/LmsApiService'; -import StatusAlert from '../StatusAlert'; import SUBMIT_STATES from '../../data/constants/formSubmissions'; import { handleErrors, validateLmsConfigForm } from './common'; @@ -152,13 +153,16 @@ function CornerstoneIntegrationConfigForm({ enterpriseId, config }) { if (state.error) { errorAlert = (
- + > + + Unable to submit config form: + +

{state.error}

+
); } diff --git a/src/components/LmsConfigurations/DegreedIntegrationConfigForm.jsx b/src/components/LmsConfigurations/DegreedIntegrationConfigForm.jsx index 9ba5208a45..ce0470872d 100644 --- a/src/components/LmsConfigurations/DegreedIntegrationConfigForm.jsx +++ b/src/components/LmsConfigurations/DegreedIntegrationConfigForm.jsx @@ -2,11 +2,12 @@ import React, { useState } from 'react'; import PropTypes from 'prop-types'; import isEmpty from 'lodash/isEmpty'; import { - ValidationFormGroup, Input, StatefulButton, Icon, + ValidationFormGroup, Input, StatefulButton, Icon, Alert, } from '@edx/paragon'; +import { Error } from '@edx/paragon/icons'; + import { snakeCaseFormData } from '../../utils'; import LmsApiService from '../../data/services/LmsApiService'; -import StatusAlert from '../StatusAlert'; import SUBMIT_STATES from '../../data/constants/formSubmissions'; import { handleErrors, validateLmsConfigForm } from './common'; @@ -157,13 +158,14 @@ function DegreedIntegrationConfigForm({ enterpriseId, config }) { if (state.error) { errorAlert = (
- + > + Unable to submit config form: +

{state.error}

+
); } diff --git a/src/components/LmsConfigurations/MoodleIntegrationConfigForm.jsx b/src/components/LmsConfigurations/MoodleIntegrationConfigForm.jsx index f7495ae560..cf345d1288 100644 --- a/src/components/LmsConfigurations/MoodleIntegrationConfigForm.jsx +++ b/src/components/LmsConfigurations/MoodleIntegrationConfigForm.jsx @@ -2,11 +2,12 @@ import React from 'react'; import PropTypes from 'prop-types'; import isEmpty from 'lodash/isEmpty'; import { - ValidationFormGroup, Input, StatefulButton, Icon, + ValidationFormGroup, Input, StatefulButton, Icon, Alert, } from '@edx/paragon'; +import { Error } from '@edx/paragon/icons'; + import { snakeCaseFormData } from '../../utils'; import LmsApiService from '../../data/services/LmsApiService'; -import StatusAlert from '../StatusAlert'; import SUBMIT_STATES from '../../data/constants/formSubmissions'; import { handleErrors, validateLmsConfigForm } from './common'; @@ -116,12 +117,13 @@ class MoodleIntegrationConfigForm extends React.Component { if (error) { errorAlert = (
- + + Unable to submit config form: +

{error}

+
); } diff --git a/src/components/LmsConfigurations/SuccessFactorsIntegrationConfigForm.jsx b/src/components/LmsConfigurations/SuccessFactorsIntegrationConfigForm.jsx index 807a76bfbc..c1b5a4cb10 100644 --- a/src/components/LmsConfigurations/SuccessFactorsIntegrationConfigForm.jsx +++ b/src/components/LmsConfigurations/SuccessFactorsIntegrationConfigForm.jsx @@ -2,11 +2,12 @@ import React from 'react'; import PropTypes from 'prop-types'; import isEmpty from 'lodash/isEmpty'; import { - ValidationFormGroup, Input, StatefulButton, Icon, + ValidationFormGroup, Input, StatefulButton, Icon, Alert, } from '@edx/paragon'; +import { Error } from '@edx/paragon/icons'; + import { snakeCaseFormData } from '../../utils'; import LmsApiService from '../../data/services/LmsApiService'; -import StatusAlert from '../StatusAlert'; import SUBMIT_STATES from '../../data/constants/formSubmissions'; import { handleErrors, validateLmsConfigForm } from './common'; @@ -317,12 +318,13 @@ class SuccessFactorsIntegrationConfigForm extends React.Component {
- + + Unable to submit config form: +

{error}

+
diff --git a/src/components/LmsConfigurations/SuccessFactorsIntegrationConfigForm.test.jsx b/src/components/LmsConfigurations/SuccessFactorsIntegrationConfigForm.test.jsx index 040a2989b1..3cdf55e2c2 100644 --- a/src/components/LmsConfigurations/SuccessFactorsIntegrationConfigForm.test.jsx +++ b/src/components/LmsConfigurations/SuccessFactorsIntegrationConfigForm.test.jsx @@ -38,9 +38,9 @@ describe('', () => { expect(spyUpdate).toHaveBeenCalledWith(formData, config.id); }); - it('show StatusAlert on errors', () => { + it('show Alert on errors', () => { const wrapper = mount(()); wrapper.setState({ error: 'error occurred.' }); - expect(wrapper.find('StatusAlert').first().props().message).toEqual('error occurred.'); + expect(wrapper.find('Alert').find('AlertHeading').first().text()).toEqual('Unable to submit config form:'); }); }); diff --git a/src/components/RegisteredLearnersTable/__snapshots__/RegisteredLearnersTable.test.jsx.snap b/src/components/RegisteredLearnersTable/__snapshots__/RegisteredLearnersTable.test.jsx.snap index d5e6d130e9..41ad7ef358 100644 --- a/src/components/RegisteredLearnersTable/__snapshots__/RegisteredLearnersTable.test.jsx.snap +++ b/src/components/RegisteredLearnersTable/__snapshots__/RegisteredLearnersTable.test.jsx.snap @@ -3,21 +3,30 @@ exports[`RegisteredLearnersTable renders empty state correctly 1`] = `
-
- -
+ +
-
-
- There are no results. -
-
+ There are no results.
diff --git a/src/components/RequestCodesPage/RequestCodesForm.jsx b/src/components/RequestCodesPage/RequestCodesForm.jsx index 1748caf8db..6dbfc7bb1d 100644 --- a/src/components/RequestCodesPage/RequestCodesForm.jsx +++ b/src/components/RequestCodesPage/RequestCodesForm.jsx @@ -2,10 +2,10 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Field, reduxForm } from 'redux-form'; import { Link, Redirect } from 'react-router-dom'; -import { Button, Icon } from '@edx/paragon'; +import { Button, Icon, Alert } from '@edx/paragon'; +import { Error } from '@edx/paragon/icons'; import RenderField from '../RenderField'; -import StatusAlert from '../StatusAlert'; import { isRequired, isValidEmail, isValidNumber, maxLength512, @@ -29,13 +29,14 @@ class RequestCodesForm extends React.Component { const { error: { message } } = this.props; return ( - + variant="danger" + icon={Error} + > + Unable to request more codes +

{`Try refreshing your screen (${message})`}

+ ); } diff --git a/src/components/SamlProviderConfiguration/SamlProviderConfigForm.jsx b/src/components/SamlProviderConfiguration/SamlProviderConfigForm.jsx index fd7188d833..23003d55f0 100644 --- a/src/components/SamlProviderConfiguration/SamlProviderConfigForm.jsx +++ b/src/components/SamlProviderConfiguration/SamlProviderConfigForm.jsx @@ -2,9 +2,9 @@ import React from 'react'; import PropTypes from 'prop-types'; import isEmpty from 'lodash/isEmpty'; import { - ValidationFormGroup, Input, StatefulButton, Icon, Button, + ValidationFormGroup, Input, StatefulButton, Icon, Button, Alert, } from '@edx/paragon'; -import StatusAlert from '../StatusAlert'; +import { Error } from '@edx/paragon/icons'; import SamlConfiguration from '../SamlConfiguration'; import SUBMIT_STATES from '../../data/constants/formSubmissions'; @@ -89,12 +89,13 @@ class SamlProviderConfigForm extends React.Component { if (error) { errorAlert = (
- + + Unable to submit config form: +

{error}

+
); } diff --git a/src/components/SamlProviderConfiguration/SamlProviderDataForm.jsx b/src/components/SamlProviderConfiguration/SamlProviderDataForm.jsx index 95a2f09476..29600eba80 100644 --- a/src/components/SamlProviderConfiguration/SamlProviderDataForm.jsx +++ b/src/components/SamlProviderConfiguration/SamlProviderDataForm.jsx @@ -2,9 +2,10 @@ import React from 'react'; import PropTypes from 'prop-types'; import isEmpty from 'lodash/isEmpty'; import { - ValidationFormGroup, Input, StatefulButton, Icon, Button, + ValidationFormGroup, Input, StatefulButton, Icon, Button, Alert, } from '@edx/paragon'; -import StatusAlert from '../StatusAlert'; +import { Error } from '@edx/paragon/icons'; + import SUBMIT_STATES from '../../data/constants/formSubmissions'; export const REQUIRED_DATA_FIELDS = [ @@ -79,12 +80,13 @@ class SamlProviderDataForm extends React.Component { if (error) { errorAlert = (
- + + Unable to submit Data form: +

{error}

+
); } diff --git a/src/components/StatusAlert/_StatusAlert.scss b/src/components/StatusAlert/_StatusAlert.scss deleted file mode 100644 index 53582680ea..0000000000 --- a/src/components/StatusAlert/_StatusAlert.scss +++ /dev/null @@ -1,12 +0,0 @@ -.alert { - .icon { - .fa { - font-size: $font-size-lg; - } - } - - .title { - font-weight: 600; - display: block; - } -} diff --git a/src/components/StatusAlert/index.jsx b/src/components/StatusAlert/index.jsx deleted file mode 100644 index 0396e573a6..0000000000 --- a/src/components/StatusAlert/index.jsx +++ /dev/null @@ -1,61 +0,0 @@ -import React from 'react'; -import PropTypes from 'prop-types'; -import { Alert, Icon } from '@edx/paragon'; - -const StatusAlert = (props) => { - const { - alertType, - className, - iconClassName, - title, - message, - dismissible, - onClose, - } = props; - - const showIcon = () => ( -
- -
- ); - - return ( - -
-
- {title && ( - {title} - )} - {message} -
-
- - ); -}; - -StatusAlert.propTypes = { - alertType: PropTypes.string.isRequired, - message: PropTypes.oneOfType([PropTypes.string, PropTypes.element]).isRequired, - className: PropTypes.string, - iconClassName: PropTypes.string, - title: PropTypes.string, - dismissible: PropTypes.bool, - onClose: PropTypes.func, -}; - -StatusAlert.defaultProps = { - className: '', - iconClassName: undefined, - title: null, - dismissible: false, - onClose: () => {}, -}; - -export default StatusAlert; diff --git a/src/components/TableComponent/index.jsx b/src/components/TableComponent/index.jsx index 97a7f9af0c..9394e4d108 100644 --- a/src/components/TableComponent/index.jsx +++ b/src/components/TableComponent/index.jsx @@ -2,12 +2,12 @@ import React from 'react'; import PropTypes from 'prop-types'; import { sendEnterpriseTrackEvent } from '@edx/frontend-enterprise-utils'; -import { Pagination, DataTable } from '@edx/paragon'; +import { Pagination, DataTable, Alert } from '@edx/paragon'; +import { Error, WarningFilled } from '@edx/paragon/icons'; import 'font-awesome/css/font-awesome.css'; import TableLoadingSkeleton from './TableLoadingSkeleton'; import TableLoadingOverlay from '../TableLoadingOverlay'; -import StatusAlert from '../StatusAlert'; import { updateUrl } from '../../utils'; class TableComponent extends React.Component { @@ -125,22 +125,24 @@ class TableComponent extends React.Component { renderErrorMessage() { return ( - + + Unable to load data +

{`Try refreshing your screen (${this.props.error.message})`}

+ ); } renderEmptyDataMessage() { return ( - + + There are no results. + ); } diff --git a/src/components/analytics/AnalyticsPage.jsx b/src/components/analytics/AnalyticsPage.jsx index 6afe0e3a18..6aa0a523ca 100644 --- a/src/components/analytics/AnalyticsPage.jsx +++ b/src/components/analytics/AnalyticsPage.jsx @@ -2,9 +2,10 @@ import React, { useState } from 'react'; import { connect } from 'react-redux'; import PropTypes from 'prop-types'; import { Helmet } from 'react-helmet'; +import { Alert } from '@edx/paragon'; +import { CheckCircle, Error } from '@edx/paragon/icons'; import Hero from '../Hero'; -import StatusAlert from '../StatusAlert'; import AnalyticsCharts from './AnalyticsCharts'; const PAGE_TITLE = 'Analytics'; @@ -24,14 +25,18 @@ function AnalyticsPage({ enterpriseId }) { const renderStatusMessage = () => ( status && status.visible && ( - setSuccessStatus({ visible: false })} dismissible - /> + > + + {status.title} + + {status.message} + + ) ); diff --git a/src/containers/CouponDetails/CouponDetails.test.jsx b/src/containers/CouponDetails/CouponDetails.test.jsx index 41782f45f8..77746279da 100644 --- a/src/containers/CouponDetails/CouponDetails.test.jsx +++ b/src/containers/CouponDetails/CouponDetails.test.jsx @@ -10,7 +10,7 @@ import { mount } from 'enzyme'; import { render, screen } from '@testing-library/react'; import '@testing-library/jest-dom/extend-expect'; -import { Alert as StatusAlert } from '@edx/paragon'; +import { Alert } from '@edx/paragon'; import { SINGLE_USE, MULTI_USE, ONCE_PER_CUSTOMER } from '../../data/constants/coupons'; import EcommerceaApiService from '../../data/services/EcommerceApiService'; @@ -338,8 +338,8 @@ describe('CouponDetails container', () => { /> )); - expect(wrapper.find(StatusAlert).prop('variant')).toEqual('danger'); - wrapper.find(StatusAlert).find('.alert-dialog .btn').simulate('click'); // Retry fetching coupon overview data + expect(wrapper.find(Alert).prop('variant')).toEqual('danger'); + wrapper.find(Alert).find('#try-again').find('.btn').simulate('click'); // Retry fetching coupon overview data expect(spy).toBeCalledTimes(1); expect(spy).toBeCalledWith({ coupon_id: initialCouponData.id }); @@ -504,13 +504,13 @@ describe('CouponDetails container', () => { wrapper.update(); // success status alert - const statusAlert = wrapper.find(StatusAlert); + const statusAlert = wrapper.find(Alert); expect(statusAlert.prop('variant')).toEqual('success'); expect(statusAlert.text()).toContain(SUCCESS_MESSAGES.assign); - statusAlert.find('.alert-dialog .btn').simulate('click'); + statusAlert.find({ children: 'Dismiss' }).simulate('click'); // after alert is dismissed - expect(wrapper.find(StatusAlert)).toHaveLength(0); + expect(wrapper.find(Alert)).toHaveLength(0); expect(wrapper.find('CouponDetails').text()).toContain(COUPON_FILTERS.unredeemed.label); // fetches overview data for coupon @@ -529,7 +529,7 @@ describe('CouponDetails container', () => { wrapper.update(); // success status alert - const statusAlert = wrapper.find(StatusAlert); + const statusAlert = wrapper.find(Alert); expect(statusAlert.prop('variant')).toEqual('success'); expect(statusAlert.text()).toContain(SUCCESS_MESSAGES.revoke); @@ -549,7 +549,7 @@ describe('CouponDetails container', () => { wrapper.update(); // success status alert - const statusAlert = wrapper.find(StatusAlert); + const statusAlert = wrapper.find(Alert); expect(statusAlert.prop('variant')).toEqual('success'); expect(statusAlert.text()).toContain(SUCCESS_MESSAGES.remind); diff --git a/src/index.scss b/src/index.scss index 8b689cfd02..e7f2a1fa57 100644 --- a/src/index.scss +++ b/src/index.scss @@ -19,7 +19,6 @@ $fa-font-path: "~font-awesome/fonts"; @import "./components/RequestCodesPage/RequestCodesPage"; @import "./components/Sidebar/Sidebar"; @import "./components/subscriptions/styles/"; -@import "./components/StatusAlert/StatusAlert"; @import "./components/LoadingMessage/LoadingMessage"; @import "./components/TableComponent/TableComponent"; @import "./components/TableLoadingOverlay/TableLoadingOverlay"; From 208f779c5f154697252b01c39592bd4958b41110 Mon Sep 17 00:00:00 2001 From: Abdullah Waheed Date: Tue, 28 Jun 2022 16:14:51 +0500 Subject: [PATCH 11/33] fix: undo removal of coverage in package.json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3d44a5cd0b..4eb2a06c67 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "start": "fedx-scripts webpack-dev-server --progress", "start:with-theme": "THEME=npm:@edx/brand-edx.org@latest npm run install-theme && fedx-scripts webpack-dev-server --progress", "debug-test": "node --inspect-brk node_modules/.bin/jest --runInBand --coverage", - "test": "TZ=UTC fedx-scripts jest ", + "test": "TZ=UTC fedx-scripts jest --coverage --passWithNoTests --maxWorkers=50%", "test:watch": "npm run test -- --watch", "snapshot": "fedx-scripts jest --updateSnapshot" }, From 855f298acf5430687da0751669e03f4be04fcd14 Mon Sep 17 00:00:00 2001 From: Kira Miller <31229189+kiram15@users.noreply.github.com> Date: Wed, 15 Jun 2022 10:19:49 -0400 Subject: [PATCH 12/33] fix: fixing config info entry bug (#800) * fix: fixing config info entry bug * fix: fixing config info entry bug --- .../settings/SettingsSSOTab/SSOStepper.jsx | 41 ++++++++------ .../steps/SSOConfigConfigureStep.jsx | 53 +++++++++---------- .../steps/SSOConfigConfigureStep.test.jsx | 17 ++++-- .../tests/NewSSOConfigForm.test.jsx | 4 +- src/components/settings/data/constants.js | 2 +- 5 files changed, 66 insertions(+), 51 deletions(-) diff --git a/src/components/settings/SettingsSSOTab/SSOStepper.jsx b/src/components/settings/SettingsSSOTab/SSOStepper.jsx index a1403c56ba..6f3d7eac56 100644 --- a/src/components/settings/SettingsSSOTab/SSOStepper.jsx +++ b/src/components/settings/SettingsSSOTab/SSOStepper.jsx @@ -5,7 +5,9 @@ import PropTypes from 'prop-types'; import { ArrowBack, ArrowForward } from '@edx/paragon/icons'; import isEmpty from 'validator/lib/isEmpty'; import isURL from 'validator/lib/isURL'; -import React, { useContext, useMemo, useState } from 'react'; +import React, { + useContext, useMemo, useState, +} from 'react'; import { connect } from 'react-redux'; import { updateCurrentStep } from './data/actions'; import { useExistingProviderData, useIdpState } from './hooks'; @@ -33,6 +35,7 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { const [existingProviderData] = useExistingProviderData(enterpriseId, refreshBool); const [showValidatedText, setShowValidatedText] = useState(false); const [idpNextButtonDisabled, setIdpNextButtonDisabled] = useState(false); + const [configNextButtonDisabled, setConfigNextButtonDisabled] = useState(false); const { metadataURL, @@ -50,16 +53,26 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { setRefreshBool(!refreshBool); }; + async function sendData(type) { + const configFormData = new FormData(); + Object.keys(configValues).forEach(key => configFormData.append(key, configValues[key])); + configFormData.append('enterprise_customer_uuid', enterpriseId); + try { + const response = await LmsApiService.updateProviderConfig(configFormData, providerConfig.id); + if (type === 'update') { + setProviderConfig(response.data); + } + return null; + } catch (error) { + setConnectError(true); + return handleErrors(error); + } + } + const saveOnQuit = async () => { let err; if (configValues !== null) { - configValues.append('enterprise_customer_uuid', enterpriseId); - try { - await LmsApiService.updateProviderConfig(configValues, providerConfig.id); - } catch (error) { - err = handleErrors(error); - setConnectError(true); - } + err = sendData('save'); } if (!err) { handleCancel(); @@ -69,14 +82,7 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { const updateConfig = async () => { let err; if (configValues !== null) { - configValues.append('enterprise_customer_uuid', enterpriseId); - try { - const response = await LmsApiService.updateProviderConfig(configValues, providerConfig.id); - setProviderConfig(response.data); - } catch (error) { - err = handleErrors(error); - setConnectError(true); - } + err = sendData('update'); } if (!err) { setCurrentStep('connect'); @@ -111,6 +117,7 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { refreshBool={refreshBool} setRefreshBool={setRefreshBool} setFormUpdated={setFormUpdated} + setConfigNextButtonDisabled={setConfigNextButtonDisabled} /> @@ -182,7 +189,7 @@ const SSOStepper = ({ enterpriseSlug, enterpriseId, enterpriseName }) => { Cancel - + + +
  • + +
    • + + @@ -918,101 +1002,6 @@ exports[` basic rendering should render table data 1`] = ` -
    -
    - -
    -
    @@ -1109,7 +1098,7 @@ exports[` basic rendering should render table data when sea Showing 1 of - 1 + 0 . @@ -1292,12 +1281,96 @@ exports[` basic rendering should render table data when sea
    -
    - Showing - 1 - of - 1 - . +
    +
    @@ -1306,101 +1379,6 @@ exports[` basic rendering should render table data when sea -
    -
    - -
    -
    diff --git a/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap b/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap index 82f2bda231..d6f6328caf 100644 --- a/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap +++ b/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap @@ -104,7 +104,14 @@ exports[`EnrolledLearnersForInactiveCoursesTable renders enrolled learners for i > Email + + + + + Total Course Enrollment Count + + + + + Total Completed Courses Count + + + + + Last Activity Date + + + + + @@ -260,12 +384,96 @@ exports[`EnrolledLearnersForInactiveCoursesTable renders enrolled learners for i
    -
    - Showing - 3 - of - 3 - . +
    +
    @@ -274,100 +482,5 @@ exports[`EnrolledLearnersForInactiveCoursesTable renders enrolled learners for i -
    -
    - -
    -
    `; diff --git a/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap b/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap index c0fafc956e..63d83b2c41 100644 --- a/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap +++ b/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap @@ -64,7 +64,14 @@ exports[`LearnerActivityTable renders active learners table correctly 1`] = ` > Email + + + + + Course Title + + + + + Course Price + + + + + Start Date + + + + + End Date + + + + + Passed Date + + + + + Current Grade + + + + + Progress Status + + + + + Last Activity Date + + + + + @@ -307,12 +586,96 @@ exports[`LearnerActivityTable renders active learners table correctly 1`] = `
    -
    - Showing - 2 - of - 2 - . +
    +
    @@ -321,101 +684,6 @@ exports[`LearnerActivityTable renders active learners table correctly 1`] = ` -
    -
    - -
    -
    `; @@ -523,7 +791,14 @@ exports[`LearnerActivityTable renders inactive past month learners table correct > Email + + + + + Course Title + + + + + Course Price + + + + + Start Date + + + + + End Date + + + + + Passed Date + + + + + Current Grade + + + + + Progress Status + + + + + Last Activity Date + + + + + @@ -766,12 +1313,96 @@ exports[`LearnerActivityTable renders inactive past month learners table correct
    -
    - Showing - 2 - of - 2 - . +
    +
    @@ -780,101 +1411,6 @@ exports[`LearnerActivityTable renders inactive past month learners table correct -
    -
    - -
    -
    `; @@ -942,7 +1478,14 @@ exports[`LearnerActivityTable renders inactive past week learners table correctl > Email + + + + + Course Title + + + + + Course Price + + + + + Start Date + + + + + End Date + + + + + Passed Date + + + + + Current Grade + + + + + Progress Status + + + + + Last Activity Date + + + + + @@ -1185,12 +2000,96 @@ exports[`LearnerActivityTable renders inactive past week learners table correctl
    -
    - Showing - 2 - of - 2 - . +
    +
    @@ -1199,100 +2098,5 @@ exports[`LearnerActivityTable renders inactive past week learners table correctl -
    -
    - -
    -
    `; diff --git a/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap b/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap index c5d603fb8b..d4b75b2bc7 100644 --- a/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap +++ b/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap @@ -64,7 +64,14 @@ exports[`PastWeekPassedLearnersTable renders table correctly 1`] = ` > Email + + + + + Course Title + + + + + Passed Date + + + + + @@ -163,12 +256,96 @@ exports[`PastWeekPassedLearnersTable renders table correctly 1`] = `
    -
    - Showing - 2 - of - 2 - . +
    +
    @@ -177,100 +354,5 @@ exports[`PastWeekPassedLearnersTable renders table correctly 1`] = ` -
    -
    - -
    -
    `; diff --git a/src/components/SubsidyRequestManagementTable/tests/__snapshots__/SubsidyRequestManagementTable.test.jsx.snap b/src/components/SubsidyRequestManagementTable/tests/__snapshots__/SubsidyRequestManagementTable.test.jsx.snap index 8973ab0b5f..59a94d0637 100644 --- a/src/components/SubsidyRequestManagementTable/tests/__snapshots__/SubsidyRequestManagementTable.test.jsx.snap +++ b/src/components/SubsidyRequestManagementTable/tests/__snapshots__/SubsidyRequestManagementTable.test.jsx.snap @@ -77,7 +77,11 @@ exports[`SubsidyRequestManagementTable renders data in a table as expected 1`] =
    - Showing 3 of 24. + Showing + 3 + of + 24 + .
    - Showing 3 of 24. + Showing + 3 + of + 24 + .
    { const sortByColumn = useCallback(args => { const { sortBy } = args; - if (sortBy.length > 0) { + if (sortBy && sortBy.length > 0) { const column = sortBy[0]; const ordering = `${column.desc ? '-' : ''}${column.id}`; updateUrl({ From 6aa0c5178a6c6a7f6b24a8a457dc51f1975624fa Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Thu, 30 Jun 2022 15:01:58 -0400 Subject: [PATCH 28/33] feat: alerts and color variants for low/no funds remaining in Learner Credit Mgmt (#812) --- .../ContactCustomerSupportButton/index.jsx | 28 +++-- src/components/EnterpriseApp/index.jsx | 6 +- .../LearnerCreditAggregateCards.jsx | 16 ++- .../LearnerCreditManagement.jsx | 7 ++ .../OfferUtilizationAlerts.jsx | 108 ++++++++++++++++++ .../data/constants.js | 9 +- .../learner-credit-management/data/utils.js | 26 +++++ .../tests/OfferUtilizationAlerts.test.jsx | 75 ++++++++++++ src/data/services/EnterpriseDataApiService.js | 13 ++- 9 files changed, 263 insertions(+), 25 deletions(-) create mode 100644 src/components/learner-credit-management/OfferUtilizationAlerts.jsx create mode 100644 src/components/learner-credit-management/tests/OfferUtilizationAlerts.test.jsx diff --git a/src/components/ContactCustomerSupportButton/index.jsx b/src/components/ContactCustomerSupportButton/index.jsx index 24cefc9168..b0bdee42cf 100644 --- a/src/components/ContactCustomerSupportButton/index.jsx +++ b/src/components/ContactCustomerSupportButton/index.jsx @@ -5,16 +5,24 @@ import classNames from 'classnames'; import { configuration } from '../../config'; -const ContactCustomerSupportButton = ({ variant, children, ...rest }) => ( - - {children} - -); +const ContactCustomerSupportButton = ({ + variant, + children, + ...rest +}) => { + const destinationUrl = configuration.ENTERPRISE_SUPPORT_URL; + + return ( + + {children} + + ); +}; ContactCustomerSupportButton.propTypes = { children: PropTypes.node, diff --git a/src/components/EnterpriseApp/index.jsx b/src/components/EnterpriseApp/index.jsx index 76c578c57c..93e4a8a916 100644 --- a/src/components/EnterpriseApp/index.jsx +++ b/src/components/EnterpriseApp/index.jsx @@ -1,7 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Redirect } from 'react-router-dom'; -import classNames from 'classnames'; import { breakpoints, MediaQuery } from '@edx/paragon'; import { getAuthenticatedUser } from '@edx/frontend-platform/auth'; @@ -120,9 +119,6 @@ class EnterpriseApp extends React.Component { return ; } - const mutedBackgroundRoutePaths = ['learner-credit']; - const hasMutedBackground = mutedBackgroundRoutePaths.some(path => this.props.location.pathname.includes(path)); - return (
    @@ -143,7 +139,7 @@ class EnterpriseApp extends React.Component { isMobile={!matchesMediaQ} />
    ( <> @@ -47,6 +49,10 @@ const LearnerCreditAggregateCards = ({ } if (totalFunds || totalFunds === 0) { + const progressBarVariant = getProgressBarVariant({ + percentUtilized, remainingFunds, + }); + return ( <> @@ -54,7 +60,9 @@ const LearnerCreditAggregateCards = ({
    Percentage Utilized
    -
    {(percentUtilized * 100).toFixed(1)}%
    +
    + {(percentUtilized * 100).toFixed(1)}% +
    @@ -64,7 +72,9 @@ const LearnerCreditAggregateCards = ({
    Remaining Funds
    -
    ${remainingFunds.toLocaleString()}
    +
    + ${remainingFunds.toLocaleString()} +
    @@ -77,7 +87,7 @@ const LearnerCreditAggregateCards = ({ now={percentUtilized * 100} label={`$${redeemedFunds.toLocaleString()}`} progressHint="Redeemed Funds" - variant="success" + variant={progressBarVariant} threshold={100} thresholdLabel={`$${totalFunds.toLocaleString()}`} thresholdVariant="dark" diff --git a/src/components/learner-credit-management/LearnerCreditManagement.jsx b/src/components/learner-credit-management/LearnerCreditManagement.jsx index bcb87ab420..bd15917184 100644 --- a/src/components/learner-credit-management/LearnerCreditManagement.jsx +++ b/src/components/learner-credit-management/LearnerCreditManagement.jsx @@ -15,6 +15,7 @@ import LearnerCreditAggregateCards from './LearnerCreditAggregateCards'; import OfferDates from './OfferDates'; import OfferNameHeading from './OfferNameHeading'; import { useOfferSummary } from './data/hooks'; +import OfferUtilizationAlerts from './OfferUtilizationAlerts'; const LearnerCreditManagement = ({ enterpriseUUID }) => { const { offers } = useContext(EnterpriseSubsidiesContext); @@ -42,6 +43,12 @@ const LearnerCreditManagement = ({ enterpriseUUID }) => { +
    diff --git a/src/components/learner-credit-management/OfferUtilizationAlerts.jsx b/src/components/learner-credit-management/OfferUtilizationAlerts.jsx new file mode 100644 index 0000000000..9a13735f0b --- /dev/null +++ b/src/components/learner-credit-management/OfferUtilizationAlerts.jsx @@ -0,0 +1,108 @@ +import React, { useEffect, useState } from 'react'; +import PropTypes from 'prop-types'; +import { Alert } from '@edx/paragon'; +import { Info, WarningFilled } from '@edx/paragon/icons'; +import { sendEnterpriseTrackEvent } from '@edx/frontend-enterprise-utils'; + +import ContactCustomerSupportButton from '../ContactCustomerSupportButton'; +import { + LOW_REMAINING_BALANCE_PERCENT_THRESHOLD, + NO_BALANCE_REMAINING_DOLLAR_THRESHOLD, +} from './data/constants'; + +const OfferUtilizationAlerts = ({ + className, + percentUtilized, + remainingFunds, + enterpriseUUID, +}) => { + const [isWarningAlertShown, setIsWarningAlertShown] = useState(false); + + useEffect(() => { + const isWarningThesholdReached = ( + percentUtilized > LOW_REMAINING_BALANCE_PERCENT_THRESHOLD + && remainingFunds > NO_BALANCE_REMAINING_DOLLAR_THRESHOLD + ); + setIsWarningAlertShown(isWarningThesholdReached); + }, [percentUtilized, remainingFunds]); + + if (percentUtilized === undefined || remainingFunds === undefined) { + return null; + } + + const isErrorAlertShown = remainingFunds <= NO_BALANCE_REMAINING_DOLLAR_THRESHOLD; + + const handleContactSupportCTAClick = (eventName) => { + sendEnterpriseTrackEvent(enterpriseUUID, eventName); + }; + + const handleOnCloseWarningAlert = () => { + sendEnterpriseTrackEvent( + enterpriseUUID, + 'edx.ui.enterprise.admin-portal.learner-credit-management.alerts.low-remaining-funds.dismissed', + ); + setIsWarningAlertShown(false); + }; + + return ( + <> + handleContactSupportCTAClick( + 'edx.ui.admin-portal.learner-credit-management.alerts.low-remaining-funds.contact-support.clicked', + )} + />, + ]} + > + Low remaining funds +

    + Your learner credit is over {(LOW_REMAINING_BALANCE_PERCENT_THRESHOLD * 100).toFixed(1)}% utilized. To + add additional funds and avoid downtime for your learners, reach out to customer support. +

    +     + handleContactSupportCTAClick( + 'edx.ui.admin-portal.learner-credit-management.alerts.no-remaining-funds.contact-support.clicked', + )} + />, + ]} + > + No remaining funds +

    + Your learner credit no longer has funds remaining. To add additional funds and + avoid downtime for your learners, reach out to customer support. +

    +     + + ); +}; + +OfferUtilizationAlerts.propTypes = { + enterpriseUUID: PropTypes.string.isRequired, + className: PropTypes.string, + percentUtilized: PropTypes.number, + remainingFunds: PropTypes.number, +}; + +OfferUtilizationAlerts.defaultProps = { + className: undefined, + percentUtilized: undefined, + remainingFunds: undefined, +}; + +export default OfferUtilizationAlerts; diff --git a/src/components/learner-credit-management/data/constants.js b/src/components/learner-credit-management/data/constants.js index e440fb28fe..1b825d0a54 100644 --- a/src/components/learner-credit-management/data/constants.js +++ b/src/components/learner-credit-management/data/constants.js @@ -1,7 +1,14 @@ -// eslint-disable-next-line import/prefer-default-export export const API_FIELDS_BY_TABLE_COLUMN_ACCESSOR = { courseTitle: 'course_title', enrollmentDate: 'enrollment_date', userEmail: 'user_email', courseListPrice: 'course_list_price', }; + +// Percentage where messaging (e.g., Alert) on low remaining balance will begin appearing +export const LOW_REMAINING_BALANCE_PERCENT_THRESHOLD = 0.75; + +// Dollar amount remaining where messaging (e.g., Alert) on no remaining balance will begin +// appearing, as learners may not be able to redeem learner credit once the organization's +// balance reaches this threshold. +export const NO_BALANCE_REMAINING_DOLLAR_THRESHOLD = 100; diff --git a/src/components/learner-credit-management/data/utils.js b/src/components/learner-credit-management/data/utils.js index 987e7e9f74..3980dff48f 100644 --- a/src/components/learner-credit-management/data/utils.js +++ b/src/components/learner-credit-management/data/utils.js @@ -1,3 +1,8 @@ +import { + LOW_REMAINING_BALANCE_PERCENT_THRESHOLD, + NO_BALANCE_REMAINING_DOLLAR_THRESHOLD, +} from './constants'; + /** * Transforms offer summary from API for display in the UI. * @@ -35,3 +40,24 @@ export const transformUtilizationTableResults = results => results.map(result => courseListPrice: result.courseListPrice, enrollmentDate: result.enrollmentDate, })); + +/** + * Gets appropriate color variant for the annotated progress bar. + * + * @param {Number} percentUtilized A float (0.0 - 1.0) of percentage funds utilized for an offer. + * @param {Number} remainingFunds A number representing remaining funds for an offer. + * + * @returns Appropriate color variant for annotated progress bar. + */ +export const getProgressBarVariant = ({ percentUtilized, remainingFunds }) => { + let variant = 'success'; // default to green + if ( + percentUtilized > LOW_REMAINING_BALANCE_PERCENT_THRESHOLD + && remainingFunds > NO_BALANCE_REMAINING_DOLLAR_THRESHOLD + ) { + variant = 'danger'; // yellow + } else if (remainingFunds <= NO_BALANCE_REMAINING_DOLLAR_THRESHOLD) { + variant = 'error'; // red + } + return variant; +}; diff --git a/src/components/learner-credit-management/tests/OfferUtilizationAlerts.test.jsx b/src/components/learner-credit-management/tests/OfferUtilizationAlerts.test.jsx new file mode 100644 index 0000000000..80130417ca --- /dev/null +++ b/src/components/learner-credit-management/tests/OfferUtilizationAlerts.test.jsx @@ -0,0 +1,75 @@ +import React from 'react'; +import { ALERT_CLOSE_LABEL_TEXT } from '@edx/paragon'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import '@testing-library/jest-dom/extend-expect'; + +import OfferUtilizationAlerts from '../OfferUtilizationAlerts'; +import { + LOW_REMAINING_BALANCE_PERCENT_THRESHOLD, + NO_BALANCE_REMAINING_DOLLAR_THRESHOLD, +} from '../data/constants'; + +const TEST_ENTERPRISE_UUID = 'test-enterprise-uuid'; + +const OfferUtilizationAlertsWrapper = (props) => ( + + + +); + +describe('', () => { + it('does not render any alerts if percent utilized and/or remaining funds is undefined', () => { + const { container } = render( + , + ); + expect(container).toBeEmptyDOMElement(); + }); + + it('does not render any alerts if conditions are not met', () => { + const { container } = render( + , + ); + expect(container).toBeEmptyDOMElement(); + }); + + it('renders low balance alert', async () => { + const { container } = render( + , + ); + expect(screen.getByText('Low remaining funds')).toBeInTheDocument(); + expect(screen.getByText('Contact support')).toBeInTheDocument(); + + // assert alert is dismissible + const dismissBtn = screen.getByText(ALERT_CLOSE_LABEL_TEXT); + expect(dismissBtn).toBeInTheDocument(); + userEvent.click(dismissBtn); + await waitFor(() => { + expect(container).toBeEmptyDOMElement(); + }); + }); + + it('renders no balance alert', () => { + render( + , + ); + expect(screen.getByText('No remaining funds')).toBeInTheDocument(); + expect(screen.getByText('Contact support')).toBeInTheDocument(); + expect(screen.queryByText(ALERT_CLOSE_LABEL_TEXT)).not.toBeInTheDocument(); + }); +}); diff --git a/src/data/services/EnterpriseDataApiService.js b/src/data/services/EnterpriseDataApiService.js index 0443b0bc75..bf00da3de6 100644 --- a/src/data/services/EnterpriseDataApiService.js +++ b/src/data/services/EnterpriseDataApiService.js @@ -31,12 +31,13 @@ class EnterpriseDataApiService { } static fetchEnterpriseOfferSummary(enterpriseId, offerId, options = {}) { - const queryParams = new URLSearchParams({ - ...snakeCaseObject(options), - }); - - const url = `${EnterpriseDataApiService.enterpriseBaseUrl}${enterpriseId}/offers/${offerId}/?${queryParams.toString()}`; - + let url = `${EnterpriseDataApiService.enterpriseBaseUrl}${enterpriseId}/offers/${offerId}/`; + if (Object.keys(options).length) { + const queryParams = new URLSearchParams({ + ...snakeCaseObject(options), + }); + url += `?${queryParams.toString()}`; + } return EnterpriseDataApiService.apiClient().get(url); } From 4d6890c134dc8a67ff0a142cef57118a4daf3417 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Thu, 30 Jun 2022 16:43:44 -0400 Subject: [PATCH 29/33] feat: ignore null course price in learner credit mgmt table (#819) --- src/components/learner-credit-management/data/hooks.js | 1 + .../learner-credit-management/data/tests/hooks.test.js | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/learner-credit-management/data/hooks.js b/src/components/learner-credit-management/data/hooks.js index 44f75187c3..628d20a79a 100644 --- a/src/components/learner-credit-management/data/hooks.js +++ b/src/components/learner-credit-management/data/hooks.js @@ -92,6 +92,7 @@ export const useLearnerCreditAllocations = (enterpriseUUID, offerId) => { page: args.pageIndex + 1, // `DataTable` uses zero-indexed array pageSize: args.pageSize, offerId, + ignoreNullCourseListPrice: true, }; if (args.sortBy?.length > 0) { applySortByToOptions(args.sortBy, options); diff --git a/src/components/learner-credit-management/data/tests/hooks.test.js b/src/components/learner-credit-management/data/tests/hooks.test.js index 64a693fc4f..91b56368b7 100644 --- a/src/components/learner-credit-management/data/tests/hooks.test.js +++ b/src/components/learner-credit-management/data/tests/hooks.test.js @@ -117,9 +117,10 @@ describe('useLearnerCreditAllocations', () => { page: 1, pageSize: 20, offerId: mockEnterpriseOffer.id, - ordering: '-enrollment_date', + ordering: '-enrollment_date', // default sort order search: mockOfferEnrollments[0].user_email, searchCourse: mockOfferEnrollments[0].course_title, + ignoreNullCourseListPrice: true, }; expect(EnterpriseDataApiService.fetchCourseEnrollments).toHaveBeenCalledWith( TEST_ENTERPRISE_UUID, From dc04679a57226f1e266aa1958a04b2b33a4d593f Mon Sep 17 00:00:00 2001 From: Abdullah Waheed Date: Fri, 1 Jul 2022 17:44:23 +0500 Subject: [PATCH 30/33] fix: failed unit tests after backmerging from master --- .../CodeSearchResults.test.jsx | 123 ++++++++++-------- .../CodeSearchResults.test.jsx.snap | 12 +- src/components/CouponDetails/index.test.jsx | 10 +- ...edLearnersForInactiveCoursesTable.test.jsx | 17 ++- ...rnersForInactiveCoursesTable.test.jsx.snap | 6 +- .../EnterpriseApp/EnterpriseApp.test.jsx | 2 +- .../LearnerActivityTable.test.jsx | 17 ++- .../LearnerActivityTable.test.jsx.snap | 18 +-- .../CornerstoneIntegrationConfigForm.test.jsx | 28 +++- .../DegreedIntegrationConfigForm.test.jsx | 28 +++- .../PastWeekPassedLearnersTable.test.jsx | 9 +- .../PastWeekPassedLearnersTable.test.jsx.snap | 6 +- ...ubsidyRequestManagementTable.test.jsx.snap | 12 +- .../CouponDetails/CouponDetails.test.jsx | 13 +- 14 files changed, 171 insertions(+), 130 deletions(-) diff --git a/src/components/CodeSearchResults/CodeSearchResults.test.jsx b/src/components/CodeSearchResults/CodeSearchResults.test.jsx index be04bd8b12..aae70500a8 100644 --- a/src/components/CodeSearchResults/CodeSearchResults.test.jsx +++ b/src/components/CodeSearchResults/CodeSearchResults.test.jsx @@ -5,6 +5,7 @@ import { MemoryRouter } from 'react-router-dom'; import { mount } from 'enzyme'; import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; import CodeSearchResults from './index'; @@ -77,12 +78,14 @@ describe('', () => { .create(( - + + + )) @@ -105,11 +108,13 @@ describe('', () => { .create(( - + + + )) @@ -164,11 +169,13 @@ describe('', () => { .create(( - + + + )) @@ -204,11 +211,13 @@ describe('', () => { .create(( - + + + )) @@ -235,11 +244,13 @@ describe('', () => { .create(( - + + + )) @@ -262,11 +273,13 @@ describe('', () => { .create(( - + + + )) @@ -305,11 +318,13 @@ describe('', () => { const wrapper = mount(( - + + + )); @@ -360,11 +375,13 @@ describe('', () => { const wrapper = mount(( - + + + )); @@ -405,11 +422,13 @@ describe('', () => { const wrapper = mount(( - + + + )); @@ -441,11 +460,13 @@ describe('', () => { const wrapper = mount(( - + + + )); diff --git a/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap b/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap index 22037c135e..d941e19603 100644 --- a/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap +++ b/src/components/CodeSearchResults/__snapshots__/CodeSearchResults.test.jsx.snap @@ -611,11 +611,7 @@ exports[` basic rendering should render table data 1`] = `
    - Showing - 4 - of - 0 - . + Showing 4 of 0.
    basic rendering should render table data when sea
    - Showing - 1 - of - 0 - . + Showing 1 of 0.
    ( - + + + ); diff --git a/src/components/EnrolledLearnersForInactiveCoursesTable/EnrolledLearnersForInactiveCoursesTable.test.jsx b/src/components/EnrolledLearnersForInactiveCoursesTable/EnrolledLearnersForInactiveCoursesTable.test.jsx index cb8eb07fd2..a103502b17 100644 --- a/src/components/EnrolledLearnersForInactiveCoursesTable/EnrolledLearnersForInactiveCoursesTable.test.jsx +++ b/src/components/EnrolledLearnersForInactiveCoursesTable/EnrolledLearnersForInactiveCoursesTable.test.jsx @@ -5,6 +5,7 @@ import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { Provider } from 'react-redux'; import { mount } from 'enzyme'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; import EnrolledLearnersForInactiveCoursesTable from '.'; @@ -95,9 +96,11 @@ const enrolledLearnersForInactiveCoursesStore = mockStore({ const EnrolledLearnersForInactiveCoursesEmptyTableWrapper = props => ( - + + + ); @@ -105,9 +108,11 @@ const EnrolledLearnersForInactiveCoursesEmptyTableWrapper = props => ( const EnrolledLearnersForInactiveCoursesWrapper = props => ( - + + + ); diff --git a/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap b/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap index d6f6328caf..8752287883 100644 --- a/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap +++ b/src/components/EnrolledLearnersForInactiveCoursesTable/__snapshots__/EnrolledLearnersForInactiveCoursesTable.test.jsx.snap @@ -74,11 +74,7 @@ exports[`EnrolledLearnersForInactiveCoursesTable renders enrolled learners for i
    - Showing - 3 - of - 3 - . + Showing 3 of 3.
    ', () => { expect(screen.getByText('/admin/coupons/request-codes')).toBeInTheDocument(); }); it('should enable code reporting screen', () => { - render(); + render(); expect(screen.getByText('/admin/reporting')).toBeInTheDocument(); }); it('should enable code subscriptions screen', () => { diff --git a/src/components/LearnerActivityTable/LearnerActivityTable.test.jsx b/src/components/LearnerActivityTable/LearnerActivityTable.test.jsx index 4b4c0ea2be..2db1783b4f 100644 --- a/src/components/LearnerActivityTable/LearnerActivityTable.test.jsx +++ b/src/components/LearnerActivityTable/LearnerActivityTable.test.jsx @@ -5,6 +5,7 @@ import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { Provider } from 'react-redux'; import { mount } from 'enzyme'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; import LearnerActivityTable from '.'; @@ -84,9 +85,11 @@ const learnerActivityStore = mockStore({ const LearnerActivityEmptyTableWrapper = props => ( - + + + ); @@ -94,9 +97,11 @@ const LearnerActivityEmptyTableWrapper = props => ( const LearnerActivityTableWrapper = props => ( - + + + ); diff --git a/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap b/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap index 63d83b2c41..cfd71116ad 100644 --- a/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap +++ b/src/components/LearnerActivityTable/__snapshots__/LearnerActivityTable.test.jsx.snap @@ -34,11 +34,7 @@ exports[`LearnerActivityTable renders active learners table correctly 1`] = `
    - Showing - 2 - of - 2 - . + Showing 2 of 2.
    - Showing - 2 - of - 2 - . + Showing 2 of 2.
    - Showing - 2 - of - 2 - . + Showing 2 of 2.
    ({ })); const enterpriseId = 'test-enterprise'; +const mockStore = configureMockStore([thunk]); +const store = mockStore({}); + +const CornerstoneIntegrationConfigFormWrapper = props => ( + + + + + + + +); describe('', () => { test('renders Cornerstone Config Form', () => { - render(); + render(); // Verify all expected fields are present. screen.getByLabelText('Active'); screen.getByLabelText('Cornerstone Instance URL'); @@ -27,13 +45,13 @@ describe('', () => { active: true, cornerstoneBaseUrl: 'initial_url', }; - render(); + render(); expect(screen.getByLabelText('Active')).toBeChecked(); expect(screen.getByLabelText('Cornerstone Instance URL')).toHaveValue('initial_url'); }); test('required fields show as invalid when not filled in', () => { - render(); + render(); fireEvent.click(screen.getByText('Submit')); expect(screen.getByLabelText('Cornerstone Instance URL')).toHaveClass('is-invalid'); @@ -49,7 +67,7 @@ describe('', () => { enterprise_customer: enterpriseId, }; - render(); + render(); fireEvent.change(screen.getByLabelText('Cornerstone Instance URL'), { target: { value: 'testinstance' }, }); @@ -66,7 +84,7 @@ describe('', () => { cornerstoneBaseUrl: 'testinstance', }; - render(); + render(); fireEvent.change(screen.getByLabelText('Cornerstone Instance URL'), { target: { value: 'changedURL' }, }); diff --git a/src/components/LmsConfigurations/DegreedIntegrationConfigForm.test.jsx b/src/components/LmsConfigurations/DegreedIntegrationConfigForm.test.jsx index 19659d15df..beb4f26ff4 100644 --- a/src/components/LmsConfigurations/DegreedIntegrationConfigForm.test.jsx +++ b/src/components/LmsConfigurations/DegreedIntegrationConfigForm.test.jsx @@ -4,6 +4,12 @@ import { import '@testing-library/jest-dom/extend-expect'; import React from 'react'; import snakeCase from 'lodash/snakeCase'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { MemoryRouter } from 'react-router-dom'; +import { Provider } from 'react-redux'; +import configureMockStore from 'redux-mock-store'; +import thunk from 'redux-thunk'; + import DegreedIntegrationConfigForm from './DegreedIntegrationConfigForm'; import LmsApiService from '../../data/services/LmsApiService'; @@ -13,10 +19,22 @@ jest.mock('../../data/services/LmsApiService', () => ({ })); const enterpriseId = 'test-enterprise'; +const mockStore = configureMockStore([thunk]); +const store = mockStore({}); + +const DegreedIntegrationConfigFormWrapper = props => ( + + + + + + + +); describe('', () => { test('renders Degreed Config Form', () => { - render(); + render(); // Verify all expected fields are present. screen.getByLabelText('Active'); screen.getByLabelText('Degreed User ID'); @@ -37,7 +55,7 @@ describe('', () => { key: 'initial_key', secret: 'initial_secret', }; - render(); + render(); expect(screen.getByLabelText('Active')).toBeChecked(); expect(screen.getByLabelText('Degreed User ID')).toHaveValue('initial_id'); expect(screen.getByLabelText('Degreed User Password')).toHaveValue('initial_pass'); @@ -48,7 +66,7 @@ describe('', () => { }); test('required fields show as invalid when not filled in', () => { - render(); + render(); fireEvent.click(screen.getByText('Submit')); expect(screen.getByLabelText('Degreed User ID')).toHaveClass('is-invalid'); @@ -74,7 +92,7 @@ describe('', () => { enterprise_customer: enterpriseId, }; - render(); + render(); fireEvent.change(screen.getByLabelText('Degreed User ID'), { target: { value: 'testuserid' }, }); @@ -111,7 +129,7 @@ describe('', () => { secret: 'testsecret', }; - render(); + render(); fireEvent.change(screen.getByLabelText('Degreed User ID'), { target: { value: 'changedUserId' }, }); diff --git a/src/components/PastWeekPassedLearnersTable/PastWeekPassedLearnersTable.test.jsx b/src/components/PastWeekPassedLearnersTable/PastWeekPassedLearnersTable.test.jsx index a4590b162f..9b88dba991 100644 --- a/src/components/PastWeekPassedLearnersTable/PastWeekPassedLearnersTable.test.jsx +++ b/src/components/PastWeekPassedLearnersTable/PastWeekPassedLearnersTable.test.jsx @@ -5,6 +5,7 @@ import configureMockStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { Provider } from 'react-redux'; import { mount } from 'enzyme'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; import PastWeekPassedLearnersTable from '.'; @@ -51,9 +52,11 @@ const store = mockStore({ const PastWeekPassedLearnersWrapper = props => ( - + + + ); diff --git a/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap b/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap index d4b75b2bc7..44a1bfacac 100644 --- a/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap +++ b/src/components/PastWeekPassedLearnersTable/__snapshots__/PastWeekPassedLearnersTable.test.jsx.snap @@ -34,11 +34,7 @@ exports[`PastWeekPassedLearnersTable renders table correctly 1`] = `
    - Showing - 2 - of - 2 - . + Showing 2 of 2.
    - Showing - 3 - of - 24 - . + Showing 3 of 24.
    - Showing - 3 - of - 24 - . + Showing 3 of 24.
    ( - + + + ); @@ -507,7 +510,7 @@ describe('CouponDetails container', () => { const statusAlert = wrapper.find(Alert); expect(statusAlert.prop('variant')).toEqual('success'); expect(statusAlert.text()).toContain(SUCCESS_MESSAGES.assign); - statusAlert.find({ children: 'Dismiss' }).simulate('click'); + statusAlert.find('.btn-tertiary').simulate('click'); // after alert is dismissed expect(wrapper.find(Alert)).toHaveLength(0); From 03c04ae43fa267280d8aa1d3ae80d2229ad07010 Mon Sep 17 00:00:00 2001 From: Abdullah Waheed Date: Fri, 1 Jul 2022 22:34:17 +0500 Subject: [PATCH 31/33] refactor: code coverage improvement --- .../CodeSearchResults/CodeSearchResultsTable.jsx | 2 +- src/components/CouponDetails/index.jsx | 1 + src/components/CouponDetails/index.test.jsx | 11 +++++++++++ src/components/analytics/AnalyticsPage.jsx | 1 - 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/components/CodeSearchResults/CodeSearchResultsTable.jsx b/src/components/CodeSearchResults/CodeSearchResultsTable.jsx index 0eff8e2d96..d488230d9a 100644 --- a/src/components/CodeSearchResults/CodeSearchResultsTable.jsx +++ b/src/components/CodeSearchResults/CodeSearchResultsTable.jsx @@ -72,7 +72,7 @@ const searchParameter = (searchQuery) => { }; const handleTableColumns = (searchQuery) => { - const assignedToColumnIndex = tableColumns.findIndex(column => column.key === 'assignedTo'); + const assignedToColumnIndex = tableColumns.findIndex(column => column.accessor === 'assignedTo'); // If search is made by email, no need to show "Assigned To" field if (isValidEmail(searchQuery) === undefined && assignedToColumnIndex > -1) { // Remove "Assigned To" column if it already exists diff --git a/src/components/CouponDetails/index.jsx b/src/components/CouponDetails/index.jsx index 6fab5d9192..541c5e5b7d 100644 --- a/src/components/CouponDetails/index.jsx +++ b/src/components/CouponDetails/index.jsx @@ -645,6 +645,7 @@ class CouponDetails extends React.Component {