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.

Sign up for GitHub

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

Jump to bottom

Remove channels label fetching in useBaseSearch composable #12877

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
+7 −84
Open

Remove channels label fetching in useBaseSearch composable #12877

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
93be629
Remove references to channels option within inputKeys
LianaHarris360 Nov 22, 2024
9899e9b
Remove labels fetching for Channels in useBaseSearch
LianaHarris360 Nov 22, 2024
f13f47b
Remove channels searchTerms option from tests
LianaHarris360 Nov 22, 2024
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
39 changes: 3 additions & 36 deletions kolibri/plugins/learn/assets/src/composables/__tests__/useSearch.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ describe(`useSearch`, () => {
expect(get(searchTerms)).toEqual({
accessibility_labels: {},
categories: {},
channels: {},
grade_levels: {},
languages: {},
learner_needs: {},
Expand All @@ -69,7 +68,6 @@ describe(`useSearch`, () => {
expect(get(searchTerms)).toEqual({
accessibility_labels: {},
categories: {},
channels: {},
grade_levels: {},
languages: {},
learner_needs: {},
Expand All @@ -82,7 +80,6 @@ describe(`useSearch`, () => {
accessibility_labels: 'test1,test2',
keywords: 'I love paris in the springtime!',
categories: 'notatest,reallynotatest,absolutelynotatest',
channels: 'channelid1,channelid2,channelid3',
grade_levels: 'lowerprimary,uppersecondary,adult',
languages: 'ar-jk,en-pr,en-gb',
learner_needs: 'internet,pencil,rolodex',
Expand All @@ -98,11 +95,6 @@ describe(`useSearch`, () => {
reallynotatest: true,
absolutelynotatest: true,
},
channels: {
channelid1: true,
channelid2: true,
channelid3: true,
},
grade_levels: {
lowerprimary: true,
uppersecondary: true,
Expand Down Expand Up @@ -185,7 +177,6 @@ describe(`useSearch`, () => {
'learning_activities',
'categories',
'learner_needs',
'channels',
'accessibility_labels',
'languages',
'grade_levels',
Expand Down Expand Up @@ -250,7 +241,6 @@ describe(`useSearch`, () => {
const { labels, more, search } = prep({}, ref({ tree_id: 1, lft: 10, rght: 20 }));
const labelsSet = {
available: ['labels'],
channels: [],
languages: [],
};
ContentNodeResource.fetchCollection.mockReturnValue(Promise.resolve({ labels: labelsSet }));
Expand Down Expand Up @@ -288,27 +278,6 @@ describe(`useSearch`, () => {
getParams: { categories__isnull: true, max_results: 25, include_coach_content: false },
});
});
it('should ignore channels when descendant is set', () => {
const { search } = prep(
{
categories: `test1,test2`,
channels: 'test1',
},
ref({ tree_id: 1, lft: 10, rght: 20 }),
);
ContentNodeResource.fetchCollection.mockReturnValue(Promise.resolve({}));
search();
expect(ContentNodeResource.fetchCollection).toHaveBeenCalledWith({
getParams: {
categories: ['test1', 'test2'],
max_results: 25,
tree_id: 1,
lft__gt: 10,
rght__lt: 20,
include_coach_content: false,
},
});
});
it('should set keywords when defined', () => {
const { search } = prep({ keywords: `this is just a test` });
ContentNodeResource.fetchCollection.mockReturnValue(Promise.resolve({}));
Expand All @@ -325,7 +294,6 @@ describe(`useSearch`, () => {
const { labels, more, results, search } = prep({ categories: 'test1,test2' });
const expectedLabels = {
available: ['labels'],
channels: [],
languages: [],
};
const expectedMore = {
Expand Down Expand Up @@ -385,7 +353,6 @@ describe(`useSearch`, () => {
});
const expectedLabels = {
available: ['labels'],
channels: [],
languages: [],
};
const expectedMore = {
Expand Down Expand Up @@ -443,14 +410,14 @@ describe(`useSearch`, () => {
it('should not remove any other filters', () => {
const { removeFilterTag, router } = prep({
categories: 'test1,test2',
channels: 'channel1',
learning_activities: 'watch',
Copy link
Member

Choose a reason for hiding this comment

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

Curious why specifically watch here?

});
removeFilterTag({ value: 'test1', key: 'categories' });
expect(router.push).toHaveBeenCalledWith({
name,
query: {
categories: 'test2',
channels: 'channel1',
learning_activities: 'watch',
},
});
});
Expand All @@ -459,7 +426,7 @@ describe(`useSearch`, () => {
it('should remove all filters from the searchTerms', () => {
const { clearSearch, router } = prep({
categories: 'test1,test2',
channels: 'channel1',
learning_activities: 'watch',
keywords: 'this',
});
clearSearch();
Expand Down
2 changes: 1 addition & 1 deletion packages/kolibri-common/components/SearchFiltersPanel/SelectGroup.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
type: Object,
required: true,
validator(value) {
const inputKeys = ['channels', 'accessibility_labels', 'languages', 'grade_levels'];
const inputKeys = ['accessibility_labels', 'languages', 'grade_levels'];
return inputKeys.every(k => Object.prototype.hasOwnProperty.call(value, k));
},
},
Expand Down
1 change: 0 additions & 1 deletion packages/kolibri-common/components/SearchFiltersPanel/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,6 @@
const inputKeys = [
'learning_activities',
'learner_needs',
'channels',
'accessibility_labels',
'languages',
'grade_levels',
Expand Down
1 change: 0 additions & 1 deletion packages/kolibri-common/composables/__mocks__/useBaseSearch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ const MOCK_DEFAULTS = {
learning_activities: {},
categories: {},
learner_needs: {},
channels: {},
accessibility_labels: {},
languages: {},
grade_levels: {},
Expand Down
39 changes: 3 additions & 36 deletions packages/kolibri-common/composables/__tests__/useBaseSearch.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ describe(`useBaseSearch`, () => {
expect(get(searchTerms)).toEqual({
accessibility_labels: {},
categories: {},
channels: {},
grade_levels: {},
languages: {},
learner_needs: {},
Expand All @@ -69,7 +68,6 @@ describe(`useBaseSearch`, () => {
expect(get(searchTerms)).toEqual({
accessibility_labels: {},
categories: {},
channels: {},
grade_levels: {},
languages: {},
learner_needs: {},
Expand All @@ -82,7 +80,6 @@ describe(`useBaseSearch`, () => {
accessibility_labels: 'test1,test2',
keywords: 'I love paris in the springtime!',
categories: 'notatest,reallynotatest,absolutelynotatest',
channels: 'channelid1,channelid2,channelid3',
grade_levels: 'lowerprimary,uppersecondary,adult',
languages: 'ar-jk,en-pr,en-gb',
learner_needs: 'internet,pencil,rolodex',
Expand All @@ -98,11 +95,6 @@ describe(`useBaseSearch`, () => {
reallynotatest: true,
absolutelynotatest: true,
},
channels: {
channelid1: true,
channelid2: true,
channelid3: true,
},
grade_levels: {
lowerprimary: true,
uppersecondary: true,
Expand Down Expand Up @@ -185,7 +177,6 @@ describe(`useBaseSearch`, () => {
'learning_activities',
'categories',
'learner_needs',
'channels',
'accessibility_labels',
'languages',
'grade_levels',
Expand Down Expand Up @@ -250,7 +241,6 @@ describe(`useBaseSearch`, () => {
const { labels, more, search } = prep({}, ref({ tree_id: 1, lft: 10, rght: 20 }));
const labelsSet = {
available: ['labels'],
channels: [],
languages: [],
};
ContentNodeResource.fetchCollection.mockReturnValue(Promise.resolve({ labels: labelsSet }));
Expand Down Expand Up @@ -288,27 +278,6 @@ describe(`useBaseSearch`, () => {
getParams: { categories__isnull: true, max_results: 25, include_coach_content: false },
});
});
it('should ignore channels when descendant is set', () => {
const { search } = prep(
{
categories: `test1,test2`,
channels: 'test1',
},
ref({ tree_id: 1, lft: 10, rght: 20 }),
);
ContentNodeResource.fetchCollection.mockReturnValue(Promise.resolve({}));
search();
expect(ContentNodeResource.fetchCollection).toHaveBeenCalledWith({
getParams: {
categories: ['test1', 'test2'],
max_results: 25,
tree_id: 1,
lft__gt: 10,
rght__lt: 20,
include_coach_content: false,
},
});
});
it('should set keywords when defined', () => {
const { search } = prep({ keywords: `this is just a test` });
ContentNodeResource.fetchCollection.mockReturnValue(Promise.resolve({}));
Expand All @@ -325,7 +294,6 @@ describe(`useBaseSearch`, () => {
const { labels, more, results, search } = prep({ categories: 'test1,test2' });
const expectedLabels = {
available: ['labels'],
channels: [],
languages: [],
};
const expectedMore = {
Expand Down Expand Up @@ -385,7 +353,6 @@ describe(`useBaseSearch`, () => {
});
const expectedLabels = {
available: ['labels'],
channels: [],
languages: [],
};
const expectedMore = {
Expand Down Expand Up @@ -443,14 +410,14 @@ describe(`useBaseSearch`, () => {
it('should not remove any other filters', () => {
const { removeFilterTag, router } = prep({
categories: 'test1,test2',
channels: 'channel1',
learning_activities: 'watch',
});
removeFilterTag({ value: 'test1', key: 'categories' });
expect(router.push).toHaveBeenCalledWith({
name,
query: {
categories: 'test2',
channels: 'channel1',
learning_activities: 'watch',
},
});
});
Expand All @@ -459,7 +426,7 @@ describe(`useBaseSearch`, () => {
it('should remove all filters from the searchTerms', () => {
const { clearSearch, router } = prep({
categories: 'test1,test2',
channels: 'channel1',
learning_activities: 'watch',
keywords: 'this',
});
clearSearch();
Expand Down
9 changes: 0 additions & 9 deletions packages/kolibri-common/composables/useBaseSearch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ export const searchKeys = [
'learning_activities',
'categories',
'learner_needs',
'channels',
'accessibility_labels',
'languages',
'grade_levels',
Expand Down Expand Up @@ -226,7 +225,6 @@ export default function useBaseSearch({
if (searchableLabels) {
set(labels, {
...searchableLabels,
channels: searchableLabels.channels ? searchableLabels.channels.map(c => c.id) : [],
languages: searchableLabels.languages ? searchableLabels.languages.map(l => l.id) : [],
});
}
Expand Down Expand Up @@ -374,7 +372,6 @@ export default function useBaseSearch({
gradeLevelsList: _generateGradeLevelsList(labels.grade_levels || []),
accessibilityOptionsList: _generateAccessibilityOptionsList(labels.accessibility_labels),
languagesList: labels.languages || [],
channelsList: labels.channels || [],
});
})
.catch(err => logging.error('Failed to fetch search labels from remote', err))
Expand Down Expand Up @@ -414,17 +411,13 @@ export default function useBaseSearch({
const languagesList = computed(() => {
return _getGlobalLabels('languagesList', []);
});
const channelsList = computed(() => {
return _getGlobalLabels('channelsList', []);
});

provide('availableLearningActivities', learningActivitiesShown);
provide('availableLibraryCategories', libraryCategories);
provide('availableResourcesNeeded', resourcesNeeded);
provide('availableGradeLevels', gradeLevelsList);
provide('availableAccessibilityOptions', accessibilityOptionsList);
provide('availableLanguages', languagesList);
provide('availableChannels', channelsList);

// Provide an object of searchable labels
// This is a manifest of all the labels that could still be selected and produce search results
Expand Down Expand Up @@ -461,7 +454,6 @@ export function injectBaseSearch() {
const availableGradeLevels = inject('availableGradeLevels');
const availableAccessibilityOptions = inject('availableAccessibilityOptions');
const availableLanguages = inject('availableLanguages');
const availableChannels = inject('availableChannels');
const searchableLabels = inject('searchableLabels');
const activeSearchTerms = inject('activeSearchTerms');
return {
Expand All @@ -471,7 +463,6 @@ export function injectBaseSearch() {
availableGradeLevels,
availableAccessibilityOptions,
availableLanguages,
availableChannels,
searchableLabels,
activeSearchTerms,
};
Expand Down