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

Formatter for printing categories #946

Merged
merged 4 commits into from
Sep 10, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions static/js/formatters-internal.js
Original file line number Diff line number Diff line change
Expand Up @@ -566,3 +566,25 @@ export function getYoutubeUrl(videos = []) {
: null;
return youtubeVideoUrl;
}

/**
* construct a string of displayable category names based on given category ids from liveAPI
* and a mapping of category ids to names.
*
* @param {string[]} categoryIds category ids from liveAPI
* @param {Object[]} categoryMap mapping of category ids to names
tmeyer2115 marked this conversation as resolved.
Show resolved Hide resolved
* @param {string} categoryMap[].id id of a category entry
* @param {string} categoryMap[].category name of a category entry
* @returns {string} category names separated by comma
*/
export function getCategoryNames(categoryIds, categoryMap) {
if (!categoryIds || !categoryMap) {
return '';
}
const categoryNames = categoryIds.reduce((list, id) => {
const categoryEntry = categoryMap.find(category => category.id === id);
categoryEntry && list.push(categoryEntry.category);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe if there's not a corresponding category entry, we display a console.error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

return list;
}, []);
return categoryNames.join(', ');
Copy link
Contributor

@oshi97 oshi97 Sep 10, 2021

Choose a reason for hiding this comment

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

are they going to ask for the last element to be separated with an "and" instead of a comma?
like would they ever want
"category 1, category2, and category3"
(I hope not because i18n would be annoying with this)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. Talked to Rose, I will revert this to return an array of category instead, and it will be up to the user to customize it with join list formatter or manipulate the list however they want

}
6 changes: 4 additions & 2 deletions static/js/formatters.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ import {
generateCTAFieldTypeLink,
price,
highlightField,
getYoutubeUrl
getYoutubeUrl,
getCategoryNames
} from './formatters-internal.js';
import * as CustomFormatters from './formatters-custom.js';

Expand Down Expand Up @@ -62,7 +63,8 @@ let Formatters = {
generateCTAFieldTypeLink,
price,
highlightField,
getYoutubeUrl
getYoutubeUrl,
getCategoryNames
};
Formatters = Object.assign(Formatters, CustomFormatters);

Expand Down
46 changes: 46 additions & 0 deletions tests/static/js/formatters.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,4 +153,50 @@ describe('Formatters', () => {
expect(actual).toEqual(expected);
});
});

describe('getCategoryNames', () => {
const categoryMap = [
{
"id": "1",
"category": "Neurology"
},
{
"id": "2",
"category": "Dermatology"
},
{
"id": "3",
"category": "Psychiatry"
},
{
"id": "4",
"category": "Surgery"
}
];

it('Handle undefined categoryIds and categoryMap', () => {
let categoryNames = Formatters.getCategoryNames(null, categoryMap);
expect(categoryNames).toEqual('');
categoryNames = Formatters.getCategoryNames(['1'], null);
expect(categoryNames).toEqual('');
});

it('return empty list for no matching category names', () => {
const categoryIds = ['5', '0'];
const categoryNames = Formatters.getCategoryNames(categoryIds, categoryMap);
expect(categoryNames).toEqual('');
});

it('return a list of matching category names', () => {
const categoryIds = ['1', '3'];
const categoryNames = Formatters.getCategoryNames(categoryIds, categoryMap);
expect(categoryNames).toEqual('Neurology, Psychiatry');
});

it('return a list of category names given non-matching and matching ids', () => {
const categoryIds = ['1', '10', '4'];
const categoryNames = Formatters.getCategoryNames(categoryIds, categoryMap);
expect(categoryNames).toEqual('Neurology, Surgery');
});
});
yen-tt marked this conversation as resolved.
Show resolved Hide resolved
});