Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: Adding search/filtering for top-down assignment #1047

Merged
merged 14 commits into from
Oct 11, 2023
4 changes: 2 additions & 2 deletions __mocks__/react-instantsearch-dom.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ const advertised_course_run = {

/* eslint-disable camelcase */
const fakeHits = [
{ objectID: '1', aggregation_key: 'course:Bees101', title: 'bla', advertised_course_run, key: 'Bees101' },
{ objectID: '2', aggregation_key: 'course:Wasps200', title: 'blp', advertised_course_run, key: 'Wasps200' },
{ objectID: '1', aggregation_key: 'course:Bees101', title: 'bla', partners: [{ name: 'edX' }, { name: 'another_unused' }], advertised_course_run, key: 'Bees101' },
{ objectID: '2', aggregation_key: 'course:Wasps200', title: 'blp', partners: [{ name: 'edX' }, { name: 'another_unused' }], advertised_course_run, key: 'Wasps200' },
];
/* eslint-enable camelcase */

Expand Down
1 change: 0 additions & 1 deletion src/components/Admin/EmbeddedSubscription.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const defaultAppContext = {
},
};

// eslint-disable-next-line react/prop-types
const AppContextProvider = ({ children }) => (
<AppContext.Provider value={defaultAppContext}>
{children}
Expand Down
1 change: 0 additions & 1 deletion src/components/Admin/SubscriptionDetailPage.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ const initialSubsidyRequestContextValue = {
},
};

// eslint-disable-next-line react/prop-types
const AppContextProvider = ({ children }) => (
<AppContext.Provider value={defaultAppContext}>
{children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { SubsidyRequestsContext } from '../../subsidy-requests';
describe('LicenseAllocationHeader', () => {
const mockStore = configureMockStore();

// eslint-disable-next-line react/prop-types
const SubscriptionDetailContextWrapper = ({ children }) => (
// eslint-disable-next-line react/jsx-no-constructed-context-values
<SubscriptionDetailContext.Provider value={{
Expand All @@ -29,7 +28,6 @@ describe('LicenseAllocationHeader', () => {
</SubscriptionDetailContext.Provider>
);

// eslint-disable-next-line react/prop-types
const SubsidyRequestsContextWrapper = ({ children }) => (
// eslint-disable-next-line react/jsx-no-constructed-context-values
<SubsidyRequestsContext.Provider value={{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ const defaultProps = {

const refinements = {};

// eslint-disable-next-line react/prop-types
const CourseSearchWrapper = ({ value = { refinements }, props = defaultProps }) => (
<Provider store={mockStore()}>
<IntlProvider locale="en">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ const bulkEnrollWithCoursesSelectedRows = {
courses: [selectedCourses, coursesDispatch],
};

// eslint-disable-next-line react/prop-types
const BulkEnrollmentSubmitWrapper = ({ bulkEnrollInfo = defaultBulkEnrollInfo, ...props }) => (
<IntlProvider locale="en">
<BulkEnrollContext.Provider value={bulkEnrollInfo}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {

jest.mock('redux-form', () => ({
...jest.requireActual('redux-form'),
// eslint-disable-next-line react/prop-types

Field: ({ label, ...rest }) => <div {...rest}>{label}</div>,
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const searchClient = algoliasearch(
);

const ContentHighlightContentCardWrapper = ({
// eslint-disable-next-line react/prop-types

store = mockStore(initialState),
}) => {
const contextValue = useState({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ const searchClient = algoliasearch(
configuration.ALGOLIA.SEARCH_API_KEY,
);

// eslint-disable-next-line react/prop-types
const HighlightStepperConfirmContentWrapper = ({ children, currentSelectedRowIds = [] }) => {
const contextValue = useState({
stepperModal: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ const searchClient = algoliasearch(
configuration.ALGOLIA.SEARCH_API_KEY,
);

// eslint-disable-next-line react/prop-types
const HighlightStepperSelectContentSearchWrapper = ({ children, currentSelectedRowIds = [] }) => {
const contextValue = useState({
stepperModal: {
Expand Down
4 changes: 2 additions & 2 deletions src/components/ContentHighlights/data/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export const sanitizeAndParseHTML = (htmlString) => {
// Set to false before pushing PR!! otherwise set to true to enable local testing of ContentHighlights components
// Test will fail as additional check to ensure this is set to false before pushing PR
export const TEST_FLAG = false;
// Test entepriseId for Content Highlights to display card selections and confirmation
// Test enterpriseId for Content Highlights to display card selections and confirmation
export const testEnterpriseId = '943b1234-58cf-4376-b8e0-0efcbf4bfdf9';
// function that passes through enterpriseId if TEST_FLAG is false, otherwise returns local testing enterpriseId
export const ENABLE_TESTING = (enterpriseId, enableTest = TEST_FLAG) => {
Expand Down Expand Up @@ -42,7 +42,7 @@ export const TAB_TITLES = {
// Max length of highlight title in stepper
export const MAX_HIGHLIGHT_TITLE_LENGTH = 60;

// Max highlight sets per enteprise curation
// Max highlight sets per enterprise curation
export const MAX_HIGHLIGHT_SETS_PER_ENTERPRISE_CURATION = 12;

// Max number of content items per highlight set
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('constants', () => {
});
it('renders title name in string functions', () => {
const highlightTitle = 'test-title';
// eslint-disable-next-line react/prop-types

const TestComponent = ({ children }) => (
<p>
{children}
Expand Down
2 changes: 1 addition & 1 deletion src/components/EnterpriseApp/EnterpriseApp.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const EnterpriseAppContextProvider = ({ children }) => (
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
__esModule: true,
// eslint-disable-next-line react/prop-types

Route: (props) => <span>{props.path}</span>,
Switch: (props) => props.children,
Redirect: () => 'Redirect',
Expand Down
1 change: 0 additions & 1 deletion src/components/EnterpriseList/EnterpriseList.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const store = mockStore({
},
});

// eslint-disable-next-line react/prop-types
const EnterpriseListWrapper = ({ initialEntries, ...rest }) => (
<MemoryRouter initialEntries={initialEntries}>
<Provider store={store}>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,49 @@
import React from 'react';
import { InstantSearch } from 'react-instantsearch-dom';
import algoliasearch from 'algoliasearch/lite';
import { Row, Col } from '@edx/paragon';

const BudgetDetailCatalogTabContents = () => (
<Row data-testid="budget-detail-catalog-tab-contents">
<Col>
<h3>Budget Name</h3>
<p>TODO</p>
</Col>
</Row>
);
import { SearchData, SEARCH_FACET_FILTERS } from '@edx/frontend-enterprise-catalog-search';
import CatalogSearch from './search/CatalogSearch';
import { LANGUAGE_REFINEMENT, LEARNING_TYPE_REFINEMENT } from './data';
import { configuration } from '../../config';

const BudgetDetailCatalogTabContents = () => {
const language = {
attribute: LANGUAGE_REFINEMENT,
title: 'Language',
};
const learningType = {
attribute: LEARNING_TYPE_REFINEMENT,
title: 'Learning Type',
};
// Add search facet filters if they don't exist in the list yet
[language, learningType].forEach((refinement) => {
if (!SEARCH_FACET_FILTERS.some((filter) => filter.attribute === refinement.attribute)) {
SEARCH_FACET_FILTERS.push(refinement);
}
});

const searchClient = algoliasearch(
configuration.ALGOLIA.APP_ID,
configuration.ALGOLIA.SEARCH_API_KEY,
);
return (
<Row data-testid="budget-detail-catalog-tab-contents">
<Col>
<SearchData
searchFacetFilters={[...SEARCH_FACET_FILTERS]}
>
<InstantSearch
indexName={configuration.ALGOLIA.INDEX_NAME}
searchClient={searchClient}
>
<CatalogSearch />
</InstantSearch>
</SearchData>
</Col>
</Row>
);
};

export default BudgetDetailCatalogTabContents;
97 changes: 97 additions & 0 deletions src/components/learner-credit-management/cards/CourseCard.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/* eslint-disable @typescript-eslint/naming-convention */
// variables taken from algolia not in camelcase
import React from 'react';
import PropTypes from 'prop-types';

import { camelCaseObject } from '@edx/frontend-platform';
import cardFallbackImg from '@edx/brand/paragon/images/card-imagecap-fallback.png';
import {
Badge, Button, Card, Hyperlink,
} from '@edx/paragon';
import { EXEC_COURSE_TYPE } from '../data/constants';
import { formatDate } from '../data/utils';

const CourseCard = ({
onClick, original,
kiram15 marked this conversation as resolved.
Show resolved Hide resolved
}) => {
const {
title,
cardImageUrl,
courseType,
normalizedMetadata,
partners,
} = camelCaseObject(original);

let priceText;
const altText = `${title} course image`;

return (
<Card
className="course-card"
onClick={() => onClick(original)}

Check warning on line 31 in src/components/learner-credit-management/cards/CourseCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/learner-credit-management/cards/CourseCard.jsx#L31

Added line #L31 was not covered by tests
orientation="horizontal"
tabIndex="0"
>
<Card.ImageCap
src={cardImageUrl || cardFallbackImg}
fallbackSrc={cardFallbackImg}
logoSrc={partners[0]?.logo_image_url}
srcAlt={altText}
logoAlt={partners[0]?.name}
/>
<div className="card-container">
Copy link
Member

Choose a reason for hiding this comment

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

[clarification] Is it actually necessary to have custom CSS / elements to support the horizontal card as mocked? I believe you could implement the horizontal cards without any custom CSS.

See this Paragon Playground demo for an example.

image

Copy link
Member

Choose a reason for hiding this comment

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

[question] Related, is responsiveness a concern for these horizontal cards? For example, at smaller breakpoints (browser window widths), the elements in the horizontal card variants would get fairly squished together, notably in the Card.Footer.

image

I might recommend changing the Card.Footer orientation from vertical to horizontal at an appropriate breakpoint and/or updating the CardGrid and/or its Card components to become vertical cards at the small breakpoints.

<div className="section-1">
<p className="mb-1 lead font-weight-bold">{title}</p>
<p>{partners[0]?.name}</p>
{courseType === EXEC_COURSE_TYPE && (
<Badge variant="light" className="mb-4">

Check warning on line 47 in src/components/learner-credit-management/cards/CourseCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/learner-credit-management/cards/CourseCard.jsx#L47

Added line #L47 was not covered by tests
Executive Education
Copy link
Member

@adamstankiewicz adamstankiewicz Oct 5, 2023

Choose a reason for hiding this comment

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

[question] Are there supposed to be Badge elements for course types other than "Executive Education"? E.g., if it's not "Executive Education", is it supposed to say "Open Courses"?

Edit: looks like there is an example for what the Badge should say for open courses.

image

</Badge>
)}
{courseType !== EXEC_COURSE_TYPE && (
<p className="spacer" />
)}
<p className={`small ${courseType !== EXEC_COURSE_TYPE ? 'mt-5 mb-0' : ''}`}>
Starts {formatDate(normalizedMetadata?.start_date)} •
Learner must register by {formatDate(normalizedMetadata?.enroll_by_date)}
</p>
</div>
<Card.Section className="section-2">
<p className="lead font-weight-bold mb-0">{priceText}</p>
<p className="micro mb-5.5">Per learner price</p>
<Card.Footer orientation="horizontal" className="footer">
<Button as={Hyperlink} destination="https://edx.org" target="_blank">View course</Button>

<Button>Assign</Button>
</Card.Footer>
</Card.Section>
</div>
</Card>
);
};

CourseCard.defaultProps = {
onClick: () => {},

Check warning on line 74 in src/components/learner-credit-management/cards/CourseCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/learner-credit-management/cards/CourseCard.jsx#L74

Added line #L74 was not covered by tests
};

CourseCard.propTypes = {
onClick: PropTypes.func,
original: PropTypes.shape({
title: PropTypes.string,
cardImageUrl: PropTypes.string,
partners: PropTypes.arrayOf(
PropTypes.shape({
name: PropTypes.string,
logo_image_url: PropTypes.string,
}),
),
normalizedMetadata: PropTypes.shape({
startDate: PropTypes.string,
endDate: PropTypes.string,
enrollByDate: PropTypes.string,
}),
courseType: PropTypes.string,
}).isRequired,
};

export default CourseCard;
82 changes: 82 additions & 0 deletions src/components/learner-credit-management/cards/CourseCard.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import React from 'react';
import { fireEvent, render, screen } from '@testing-library/react';
import '@testing-library/jest-dom/extend-expect';

import { IntlProvider } from '@edx/frontend-platform/i18n';
import CourseCard from './CourseCard';
import { CONTENT_TYPE_COURSE, EXEC_ED_TITLE } from '../data/constants';

jest.mock('@edx/frontend-platform', () => ({
...jest.requireActual('@edx/frontend-platform'),
}));

const TEST_CATALOG = ['ayylmao'];

const originalData = {
title: 'Course Title',
card_image_url: undefined,
partners: [{ logo_image_url: '', name: 'Course Provider' }],
first_enrollable_paid_seat_price: 100,
original_image_url: '',
enterprise_catalog_query_titles: TEST_CATALOG,
advertised_course_run: { pacing_type: 'self_paced' },
};

const defaultProps = {
original: originalData,
learningType: CONTENT_TYPE_COURSE,
};

const execEdData = {
title: 'Exec Ed Course Title',
card_image_url: undefined,
partners: [{ logo_image_url: '', name: 'Course Provider' }],
first_enrollable_paid_seat_price: 100,
original_image_url: '',
enterprise_catalog_query_titles: TEST_CATALOG,
advertised_course_run: { pacing_type: 'instructor_paced' },
entitlements: [{ price: '999.00' }],
};

const execEdProps = {
original: execEdData,
learningType: EXEC_ED_TITLE,
};

describe('Course card works as expected', () => {
test('card renders as expected', () => {
render(
<IntlProvider locale="en">
<CourseCard {...defaultProps} />
</IntlProvider>,
);
expect(screen.queryByText(defaultProps.original.title)).toBeInTheDocument();
expect(
screen.queryByText(defaultProps.original.partners[0].name),
).toBeInTheDocument();
expect(screen.queryByText('Course Title')).toBeInTheDocument();
expect(screen.queryByText('Per learner price')).toBeInTheDocument();
});
test('exec ed card renders as expected', () => {
render(
<IntlProvider locale="en">
<CourseCard {...execEdProps} />
</IntlProvider>,
);
expect(screen.queryByText(execEdProps.original.title)).toBeInTheDocument();
expect(
screen.queryByText(execEdProps.original.partners[0].name),
).toBeInTheDocument();
expect(screen.queryByText('Exec Ed Course Title')).toBeInTheDocument();
});
test('test card renders default image', async () => {
render(
<IntlProvider locale="en">
<CourseCard {...defaultProps} />
</IntlProvider>,
);
const imageAltText = `${originalData.title} course image`;
fireEvent.error(screen.getByAltText(imageAltText));
await expect(screen.getByAltText(imageAltText).src).not.toBeUndefined;
});
});
10 changes: 10 additions & 0 deletions src/components/learner-credit-management/data/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,13 @@ export const BUDGET_DETAIL_TAB_LABELS = {
[BUDGET_DETAIL_ACTIVITY_TAB]: 'Activity',
[BUDGET_DETAIL_CATALOG_TAB]: 'Catalog',
};

// Facet filters
export const LEARNING_TYPE_REFINEMENT = 'learning_type';
export const LANGUAGE_REFINEMENT = 'language';

// Learning types
export const CONTENT_TYPE_COURSE = 'course';
export const EXEC_ED_TITLE = 'Executive Education';

export const EXEC_COURSE_TYPE = 'executive-education-2u';
Loading