From 9570b964c459e6f61a6c862fc77a53ba0db299a1 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Wed, 8 Oct 2025 21:50:01 +0300 Subject: [PATCH 01/11] Adds defensive check when reading file_format on file object --- .../channelEdit/views/files/ContentRenderer.vue | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/files/ContentRenderer.vue b/contentcuration/contentcuration/frontend/channelEdit/views/files/ContentRenderer.vue index b928aca6e6..b363de29b6 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/files/ContentRenderer.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/files/ContentRenderer.vue @@ -201,27 +201,30 @@ f => f.language.id, ); }, + fileFormat() { + return this.file?.file_format || ''; + }, isVideo() { - return this.file.file_format === 'mp4' || this.file.file_format === 'webm'; + return this.fileFormat === 'mp4' || this.fileFormat === 'webm'; }, isAudio() { - return this.file.file_format === 'mp3'; + return this.fileFormat === 'mp3'; }, isHTML() { - return this.file.file_format === 'zip'; + return this.fileFormat === 'zip'; }, isPDF() { - return this.file.file_format === 'pdf'; + return this.fileFormat === 'pdf'; }, isEpub() { - return this.file.file_format === 'epub'; + return this.fileFormat === 'epub'; }, isSupported() { return this.isVideo || this.isAudio || this.isHTML || this.isPDF || this.isEpub; }, htmlPath() { const entry = get(this.contentNode, ['extra_fields', 'options', 'entry'], 'index.html'); - return `/zipcontent/${this.file.checksum}.${this.file.file_format}/${entry}`; + return `/zipcontent/${this.file.checksum}.${this.fileFormat}/${entry}`; }, src() { return this.file && this.file.url; From a77e3ed287b805589458360c60cd9acaf7c75d8a Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Fri, 10 Oct 2025 16:42:01 +0300 Subject: [PATCH 02/11] Adds defensive check when validating node title --- .../contentcuration/frontend/shared/utils/validation.js | 3 ++- .../contentcuration/frontend/shared/utils/validation.spec.js | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/shared/utils/validation.js b/contentcuration/contentcuration/frontend/shared/utils/validation.js index e576d678e5..bb50c05799 100644 --- a/contentcuration/contentcuration/frontend/shared/utils/validation.js +++ b/contentcuration/contentcuration/frontend/shared/utils/validation.js @@ -268,8 +268,9 @@ export function getInvalidText(validators, value) { // Node validation // These functions return an array of error codes export function getNodeTitleErrors(node) { + const title = node.title || ''; return getTitleValidators() - .map(validator => validator(node.title)) + .map(validator => validator(title)) .filter(value => value !== true); } diff --git a/contentcuration/contentcuration/frontend/shared/utils/validation.spec.js b/contentcuration/contentcuration/frontend/shared/utils/validation.spec.js index a1868b32f9..52917d3ae8 100644 --- a/contentcuration/contentcuration/frontend/shared/utils/validation.spec.js +++ b/contentcuration/contentcuration/frontend/shared/utils/validation.spec.js @@ -62,6 +62,11 @@ describe('channelEdit utils', () => { const node = { title: ' ' }; expect(getNodeTitleErrors(node)).toEqual([ValidationErrors.TITLE_REQUIRED]); }); + + it('returns an error for a undefined title', () => { + const node = {}; + expect(getNodeTitleErrors(node)).toEqual([ValidationErrors.TITLE_REQUIRED]); + }); }); describe('getNodeLicenseErrors', () => { From 3c0b5c59a90a1e9592ff89b15c780c873a8c5c1f Mon Sep 17 00:00:00 2001 From: Michaela Robosova Date: Mon, 20 Oct 2025 05:42:59 +0200 Subject: [PATCH 03/11] Fix focus ring on some buttons (1) Remove general button style override to allow KDS focus ring logic to take effect (2) Update VBtn style override to be consistent with the KDS focus ring style --- .../contentcuration/frontend/shared/styles/main.scss | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/styles/main.scss b/contentcuration/contentcuration/frontend/shared/styles/main.scss index 8e3188950c..8a6910724c 100644 --- a/contentcuration/contentcuration/frontend/shared/styles/main.scss +++ b/contentcuration/contentcuration/frontend/shared/styles/main.scss @@ -48,9 +48,12 @@ body { text-align: center; } - .button:focus-visible, .v-btn:focus-visible { - outline: 2px solid var(--v-secondary-base) !important; + // ensures that until KDS migration is complete, + // Vuetify-based buttons have a visible focus style + // consistent with KDS buttons and links + outline: 3px solid #33acf5 !important; + outline-offset: 4px !important; } > .v-dialog__content, From 3db707fdeaf3d64eb67ae1b47af328c55129f481 Mon Sep 17 00:00:00 2001 From: Michaela Robosova Date: Mon, 20 Oct 2025 05:43:05 +0200 Subject: [PATCH 04/11] Fix confirmation dialogs not appearing for channel sharing operations. Clicking 'Delete invitation', 'Grant edit permissions', and 'Revoke view permission' did nothing. The problem may be related to stacking order of Vuetify-based confirmation dialogs. Fixed by migrating to KDS components. Also removes internal implementation dependencies from the related test suite and migrates it to Vue Testing Library. --- .../views/channel/ChannelSharingTable.vue | 201 ++++------- .../__tests__/channelSharingTable.spec.js | 338 +++++++++++++----- 2 files changed, 334 insertions(+), 205 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/channel/ChannelSharingTable.vue b/contentcuration/contentcuration/frontend/shared/views/channel/ChannelSharingTable.vue index 24f7e7f011..c3cbd26ecd 100644 --- a/contentcuration/contentcuration/frontend/shared/views/channel/ChannelSharingTable.vue +++ b/contentcuration/contentcuration/frontend/shared/views/channel/ChannelSharingTable.vue @@ -34,136 +34,61 @@ - - - - - - + {{ + $tr('removeViewerText', { first_name: selected.first_name, last_name: selected.last_name }) + }} + - - - - + {{ $tr('deleteInvitationText', { email: selected.email }) }} + - - - - + {{ + $tr('makeEditorText', { first_name: selected.first_name, last_name: selected.last_name }) + }} + @@ -173,13 +98,9 @@ import { mapActions, mapGetters, mapState } from 'vuex'; import { SharingPermissions } from 'shared/constants'; - import MessageDialog from 'shared/views/MessageDialog'; export default { name: 'ChannelSharingTable', - components: { - MessageDialog, - }, props: { channelId: { type: String, @@ -246,6 +167,40 @@ 'makeEditor', 'removeViewer', ]), + getMenuOptions(item) { + if (item.pending) { + return [ + { id: 'resend-invitation', label: this.$tr('resendInvitation'), item }, + { id: 'delete-invitation', label: this.$tr('deleteInvitation'), item }, + ]; + } else { + return [ + { id: 'make-editor', label: this.$tr('makeEditor'), item }, + { id: 'remove-viewer', label: this.$tr('removeViewer'), item }, + ]; + } + }, + onMenuSelect(selection) { + switch (selection.id) { + case 'resend-invitation': + this.resendInvitation(selection.item.email); + break; + case 'delete-invitation': + this.selected = selection.item; + this.showDeleteInvitation = true; + break; + case 'make-editor': + this.selected = selection.item; + this.showMakeEditor = true; + break; + case 'remove-viewer': + this.selected = selection.item; + this.showRemoveViewer = true; + break; + default: + break; + } + }, getUserText(user) { const nameParams = { first_name: user.first_name, diff --git a/contentcuration/contentcuration/frontend/shared/views/channel/__tests__/channelSharingTable.spec.js b/contentcuration/contentcuration/frontend/shared/views/channel/__tests__/channelSharingTable.spec.js index 5b50e04c02..e12f95fd87 100644 --- a/contentcuration/contentcuration/frontend/shared/views/channel/__tests__/channelSharingTable.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/channel/__tests__/channelSharingTable.spec.js @@ -1,118 +1,292 @@ -import { mount } from '@vue/test-utils'; -import ChannelSharingTable from './../ChannelSharingTable'; -import storeFactory from 'shared/vuex/baseStore'; +import { render, screen } from '@testing-library/vue'; +import userEvent from '@testing-library/user-event'; +import { createLocalVue } from '@vue/test-utils'; +import Vuex from 'vuex'; +import VueRouter from 'vue-router'; +import ChannelSharingTable from '../ChannelSharingTable'; import { SharingPermissions } from 'shared/constants'; -const store = storeFactory(); -const currentUser = { id: 'testId' }; -store.state.session.currentUser = currentUser; +const localVue = createLocalVue(); +localVue.use(Vuex); const channelId = 'test-channel'; -const channelUsers = [{ id: 'other-user' }, currentUser]; -const channelInvitations = [{ id: 'invitation-test' }]; -function makeWrapper(users, invites) { - return mount(ChannelSharingTable, { - store, - propsData: { - channelId, - mode: SharingPermissions.EDIT, - }, - computed: { - getChannelUsers() { - return () => users || channelUsers; +const currentUser = { + id: 'current-user-id', + email: 'current@example.com', + first_name: 'Current', + last_name: 'User', +}; +const otherUser = { + id: 'other-user-id', + email: 'other@example.com', + first_name: 'Other', + last_name: 'User', +}; +const pendingInvitation = { + id: 'pending-invitation', + email: 'pending@example.com', + first_name: 'Pending', + last_name: 'User', + pending: true, + declined: false, +}; + +const mockActions = { + sendInvitation: jest.fn(() => Promise.resolve()), + deleteInvitation: jest.fn(() => Promise.resolve()), + makeEditor: jest.fn(() => Promise.resolve()), + removeViewer: jest.fn(() => Promise.resolve()), + showSnackbar: jest.fn(() => Promise.resolve()), +}; + +const createMockStore = (users = [currentUser, otherUser], invitations = [pendingInvitation]) => { + return new Vuex.Store({ + state: { + session: { + currentUser, }, - getChannelInvitations() { - return () => invites || channelInvitations; + }, + modules: { + channel: { + namespaced: true, + actions: { + sendInvitation: mockActions.sendInvitation, + deleteInvitation: mockActions.deleteInvitation, + makeEditor: mockActions.makeEditor, + removeViewer: mockActions.removeViewer, + }, + getters: { + getChannelUsers: () => () => users, + getChannelInvitations: () => () => invitations, + }, }, }, + actions: { + showSnackbar: mockActions.showSnackbar, + }, }); -} +}; + +const renderComponent = (props = {}, storeOverrides = {}) => { + const defaultProps = { + channelId, + mode: SharingPermissions.VIEW_ONLY, + ...props, + }; -describe('channelSharingTable', () => { - let wrapper; + const store = createMockStore(storeOverrides.users, storeOverrides.invitations); + return render(ChannelSharingTable, { + localVue, + store, + props: defaultProps, + routes: new VueRouter(), + }); +}; + +describe('ChannelSharingTable', () => { beforeEach(() => { - wrapper = makeWrapper(); + jest.clearAllMocks(); + }); + + it('shows "No users found" message when no users exist', () => { + renderComponent({}, { users: [], invitations: [] }); + expect(screen.getByText('No users found')).toBeInTheDocument(); + }); + + it('displays current user correctly', () => { + renderComponent(); + const cell = screen.getByText('Current User (you)'); + expect(cell).toBeInTheDocument(); + const row = cell.closest('tr'); + expect(row).toHaveTextContent('current@example.com'); + expect(row).not.toHaveTextContent('Invite pending'); }); - it('should add current user to top of list', () => { - expect(wrapper.vm.users[0].id).toBe(currentUser.id); - expect(wrapper.vm.users[1].id).toBe(channelUsers[0].id); + it('displays other user correctly', () => { + renderComponent(); + const cell = screen.getByText('Other User'); + expect(cell).toBeInTheDocument(); + const row = cell.closest('tr'); + expect(row).toHaveTextContent('other@example.com'); + expect(row).not.toHaveTextContent('Invite pending'); }); - it('should set all invitations as pending', () => { - expect(wrapper.vm.invitations[0].pending).toBe(true); + it('displays pending user correctly', () => { + renderComponent(); + const cell = screen.getByText('Pending User'); + expect(cell).toBeInTheDocument(); + const row = cell.closest('tr'); + expect(row).toHaveTextContent('pending@example.com'); + expect(row).toHaveTextContent('Invite pending'); }); - describe('confirmation modals', () => { - it('clicking make editor option should open makeEditor confirmation modal', async () => { - await wrapper.setProps({ mode: SharingPermissions.VIEW_ONLY }); - await wrapper.findComponent('[data-test="makeeditor"]').trigger('click'); - expect(wrapper.vm.showMakeEditor).toBe(true); + describe('in edit mode', () => { + it('displays the correct header', () => { + renderComponent({ mode: SharingPermissions.EDIT }); + expect(screen.getByText(/users who can edit/)).toBeInTheDocument(); }); - it('clicking remove viewer option should open removeViewer confirmation modal', async () => { - await wrapper.setProps({ mode: SharingPermissions.VIEW_ONLY }); - await wrapper.findComponent('[data-test="removeviewer"]').trigger('click'); - expect(wrapper.vm.showRemoveViewer).toBe(true); + it('does not display the options dropdown for the current user', () => { + renderComponent({ mode: SharingPermissions.EDIT }); + const row = screen.getByText('Current User (you)').closest('tr'); + const optionsButton = row.querySelector('button'); + expect(optionsButton).not.toBeInTheDocument(); + }); + + it('does not display the options dropdown for users who accepted invitations', () => { + renderComponent({ mode: SharingPermissions.EDIT }); + const row = screen.getByText('Other User').closest('tr'); + const optionsButton = row.querySelector('button'); + expect(optionsButton).not.toBeInTheDocument(); + }); + + it('displays the options dropdown with correct options for pending invitations', async () => { + renderComponent({ mode: SharingPermissions.EDIT }); + const row = screen.getByText('Pending User').closest('tr'); + const optionsButton = row.querySelector('button'); + expect(optionsButton).toBeInTheDocument(); + + await userEvent.click(optionsButton); + + expect(screen.getByText('Resend invitation')).toBeInTheDocument(); + expect(screen.getByText('Delete invitation')).toBeInTheDocument(); + expect(screen.queryByText('Grant edit permissions')).not.toBeInTheDocument(); + expect(screen.queryByText('Revoke view permissions')).not.toBeInTheDocument(); }); }); - describe('actions', () => { - const invite = { - id: 'test-invitation', - email: 'test@testing.com', - declined: false, - }; - const user = { - id: 'test-user', - }; - - beforeEach(() => { - wrapper = makeWrapper([user], [invite]); + describe('in view-only mode', () => { + it('displays the correct header', () => { + renderComponent({ mode: SharingPermissions.VIEW_ONLY }); + expect(screen.getByText(/users who can view/)).toBeInTheDocument(); }); - it('resendInvitation should call sendInvitation', async () => { - const sendInvitation = jest.spyOn(wrapper.vm, 'sendInvitation'); - sendInvitation.mockImplementation(() => Promise.resolve()); - await wrapper.find('[data-test="resend"]').trigger('click'); - expect(sendInvitation).toHaveBeenCalledWith({ - email: invite.email, - channelId, - shareMode: SharingPermissions.EDIT, - }); + it('displays the options dropdown with correct options for pending invitations', async () => { + renderComponent({ mode: SharingPermissions.VIEW_ONLY }); + const row = screen.getByText('Pending User').closest('tr'); + const optionsButton = row.querySelector('button'); + expect(optionsButton).toBeInTheDocument(); + + await userEvent.click(optionsButton); + + expect(screen.getByText('Resend invitation')).toBeInTheDocument(); + expect(screen.getByText('Delete invitation')).toBeInTheDocument(); + expect(screen.queryByText('Grant edit permissions')).not.toBeInTheDocument(); + expect(screen.queryByText('Revoke view permissions')).not.toBeInTheDocument(); }); - it('handleDelete should call deleteInvitation', async () => { - await wrapper.setData({ selected: invite }); - const deleteInvitation = jest.spyOn(wrapper.vm, 'deleteInvitation'); - deleteInvitation.mockImplementation(() => Promise.resolve()); - await wrapper.findComponent('[data-test="confirm-delete"]').trigger('click'); - expect(deleteInvitation).toHaveBeenCalledWith(invite.id); + it('displays the options dropdown with correct options for users who accepted invitations', async () => { + renderComponent({ mode: SharingPermissions.VIEW_ONLY }); + const row = screen.getByText('Other User').closest('tr'); + const optionsButton = row.querySelector('button'); + expect(optionsButton).toBeInTheDocument(); + + await userEvent.click(optionsButton); + + expect(screen.getByText('Grant edit permissions')).toBeInTheDocument(); + expect(screen.getByText('Revoke view permissions')).toBeInTheDocument(); + expect(screen.queryByText('Resend invitation')).not.toBeInTheDocument(); + expect(screen.queryByText('Delete invitation')).not.toBeInTheDocument(); }); + }); + + describe(`clicking 'Resend invitation' menu option`, () => { + it('should resend the invitation and show a success message', async () => { + const user = userEvent.setup(); - it('clicking delete option should open delete confirmation modal', async () => { - await wrapper.findComponent('[data-test="delete"]').trigger('click'); - expect(wrapper.vm.showDeleteInvitation).toBe(true); + renderComponent(); + const row = screen.getByText('Pending User').closest('tr'); + const optionsButton = row.querySelector('button'); + + await userEvent.click(optionsButton); + const resendOption = screen.getByText('Resend invitation'); + await user.click(resendOption); + + expect(mockActions.sendInvitation).toHaveBeenCalledWith(expect.any(Object), { + email: 'pending@example.com', + shareMode: SharingPermissions.VIEW_ONLY, + channelId: channelId, + }); }); + }); + + describe(`clicking 'Delete invitation' menu option`, () => { + it('should open confirmation modal and delete invitation on confirm', async () => { + const dialogMessage = `Are you sure you would like to delete the invitation for pending@example.com?`; + const user = userEvent.setup(); + + renderComponent(); + const row = screen.getByText('Pending User').closest('tr'); + const optionsButton = row.querySelector('button'); + + expect(screen.queryByText(dialogMessage)).not.toBeInTheDocument(); - it('grantEditAccess should call makeEditor', async () => { - await wrapper.setData({ selected: user }); - await wrapper.setProps({ mode: SharingPermissions.VIEW_ONLY }); - const makeEditor = jest.spyOn(wrapper.vm, 'makeEditor'); - makeEditor.mockImplementation(() => Promise.resolve()); - await wrapper.findComponent('[data-test="confirm-makeeditor"]').trigger('click'); - expect(makeEditor).toHaveBeenCalledWith({ userId: user.id, channelId }); + await userEvent.click(optionsButton); + const deleteOption = screen.getByText('Delete invitation'); + await user.click(deleteOption); + + expect(screen.getByText(dialogMessage)).toBeInTheDocument(); + const confirmButton = screen.getByRole('button', { name: 'Delete invitation' }); + await user.click(confirmButton); + + expect(mockActions.deleteInvitation).toHaveBeenCalledWith( + expect.any(Object), + pendingInvitation.id, + ); }); + }); + + describe(`clicking 'Grant edit permissions' menu option`, () => { + it('should open confirmation modal and upgrade user on confirm', async () => { + const dialogMessage = `Are you sure you would like to grant edit permissions to Other User?`; + const user = userEvent.setup(); + + renderComponent(); + const row = screen.getByText('Other User').closest('tr'); + const optionsButton = row.querySelector('button'); + + expect(screen.queryByText(dialogMessage)).not.toBeInTheDocument(); + + await userEvent.click(optionsButton); + const grantOption = screen.getByText('Grant edit permissions'); + await user.click(grantOption); - it('handleRemoveViewer should call removeViewer', async () => { - await wrapper.setData({ selected: user }); - await wrapper.setProps({ mode: SharingPermissions.VIEW_ONLY }); - const removeViewer = jest.spyOn(wrapper.vm, 'removeViewer'); - removeViewer.mockImplementation(() => Promise.resolve()); - await wrapper.findComponent('[data-test="confirm-remove"]').trigger('click'); - expect(removeViewer).toHaveBeenCalledWith({ userId: user.id, channelId }); + expect(screen.getByText(dialogMessage)).toBeInTheDocument(); + const confirmButton = screen.getByRole('button', { name: 'Yes, grant permissions' }); + await user.click(confirmButton); + + expect(mockActions.makeEditor).toHaveBeenCalledWith(expect.any(Object), { + channelId, + userId: otherUser.id, + }); + }); + }); + + describe(`clicking 'Revoke view permissions' menu option`, () => { + it('should open confirmation modal and remove viewer on confirm', async () => { + const dialogMessage = `Are you sure you would like to revoke view permissions for Other User?`; + const user = userEvent.setup(); + + renderComponent(); + const row = screen.getByText('Other User').closest('tr'); + const optionsButton = row.querySelector('button'); + + expect(screen.queryByText(dialogMessage)).not.toBeInTheDocument(); + + await userEvent.click(optionsButton); + const revokeOption = screen.getByText('Revoke view permissions'); + await user.click(revokeOption); + + expect(screen.getByText(dialogMessage)).toBeInTheDocument(); + const confirmButton = screen.getByRole('button', { name: 'Yes, revoke' }); + await user.click(confirmButton); + + expect(mockActions.removeViewer).toHaveBeenCalledWith(expect.any(Object), { + channelId, + userId: otherUser.id, + }); }); }); }); From 36bcb936a9bd8e7350b3757d59a8c5136d411df2 Mon Sep 17 00:00:00 2001 From: Michaela Robosova Date: Mon, 20 Oct 2025 05:43:08 +0200 Subject: [PATCH 05/11] Use KButton for Send invitation --- .../frontend/shared/views/channel/ChannelSharing.vue | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/channel/ChannelSharing.vue b/contentcuration/contentcuration/frontend/shared/views/channel/ChannelSharing.vue index 9fbd995ced..f08183ddfa 100644 --- a/contentcuration/contentcuration/frontend/shared/views/channel/ChannelSharing.vue +++ b/contentcuration/contentcuration/frontend/shared/views/channel/ChannelSharing.vue @@ -59,13 +59,13 @@ - {{ $tr('inviteButton') }} - + Date: Tue, 21 Oct 2025 10:37:37 +0300 Subject: [PATCH 06/11] Streamline getNodeDetailsErrorsList and getContentNodeDetailsAreValid getters --- .../channelEdit/vuex/contentNode/getters.js | 14 +++++++++++--- .../contentcuration/frontend/shared/constants.js | 1 + .../frontend/shared/utils/validation.js | 3 +-- .../frontend/shared/utils/validation.spec.js | 5 ----- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/getters.js b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/getters.js index 2927514b80..3e31ce6ed3 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/getters.js +++ b/contentcuration/contentcuration/frontend/channelEdit/vuex/contentNode/getters.js @@ -7,7 +7,7 @@ import messages from '../../translator'; import { parseNode } from './utils'; import { getNodeDetailsErrors, getNodeFilesErrors } from 'shared/utils/validation'; import { ContentKindsNames } from 'shared/leUtils/ContentKinds'; -import { NEW_OBJECT } from 'shared/constants'; +import { NEW_OBJECT, ValidationErrors } from 'shared/constants'; import { COPYING_STATUS, COPYING_STATUS_VALUES } from 'shared/data/constants'; function sorted(nodes) { @@ -158,14 +158,22 @@ export function getContentNodeIsValid(state, getters, rootState, rootGetters) { export function getContentNodeDetailsAreValid(state) { return function (contentNodeId) { - const contentNode = state.contentNodesMap[contentNodeId]; - return contentNode && (contentNode[NEW_OBJECT] || !getNodeDetailsErrors(contentNode).length); + return !getNodeDetailsErrorsList(state)(contentNodeId).length; }; } export function getNodeDetailsErrorsList(state) { return function (contentNodeId) { const contentNode = state.contentNodesMap[contentNodeId]; + + if (!contentNode) { + return [ValidationErrors.MISSING_NODE]; + } + + if (contentNode[NEW_OBJECT]) { + return []; + } + return getNodeDetailsErrors(contentNode); }; } diff --git a/contentcuration/contentcuration/frontend/shared/constants.js b/contentcuration/contentcuration/frontend/shared/constants.js index 77ddaa9b90..f17081da03 100644 --- a/contentcuration/contentcuration/frontend/shared/constants.js +++ b/contentcuration/contentcuration/frontend/shared/constants.js @@ -190,6 +190,7 @@ export const ValidationErrors = { ACTIVITY_DURATION_MAX_FOR_LONG_ACTIVITY: 'ACTIVITY_DURATION_MAX_FOR_LONG_ACTIVITY', ACTIVITY_DURATION_MIN_REQUIREMENT: 'ACTIVITY_DURATION_MIN_REQUIREMENT', ACTIVITY_DURATION_TOO_LONG: 'ACTIVITY_DURATION_TOO_LONG', + MISSING_NODE: 'MISSING_NODE', ...fileErrors, }; diff --git a/contentcuration/contentcuration/frontend/shared/utils/validation.js b/contentcuration/contentcuration/frontend/shared/utils/validation.js index bb50c05799..e576d678e5 100644 --- a/contentcuration/contentcuration/frontend/shared/utils/validation.js +++ b/contentcuration/contentcuration/frontend/shared/utils/validation.js @@ -268,9 +268,8 @@ export function getInvalidText(validators, value) { // Node validation // These functions return an array of error codes export function getNodeTitleErrors(node) { - const title = node.title || ''; return getTitleValidators() - .map(validator => validator(title)) + .map(validator => validator(node.title)) .filter(value => value !== true); } diff --git a/contentcuration/contentcuration/frontend/shared/utils/validation.spec.js b/contentcuration/contentcuration/frontend/shared/utils/validation.spec.js index 52917d3ae8..a1868b32f9 100644 --- a/contentcuration/contentcuration/frontend/shared/utils/validation.spec.js +++ b/contentcuration/contentcuration/frontend/shared/utils/validation.spec.js @@ -62,11 +62,6 @@ describe('channelEdit utils', () => { const node = { title: ' ' }; expect(getNodeTitleErrors(node)).toEqual([ValidationErrors.TITLE_REQUIRED]); }); - - it('returns an error for a undefined title', () => { - const node = {}; - expect(getNodeTitleErrors(node)).toEqual([ValidationErrors.TITLE_REQUIRED]); - }); }); describe('getNodeLicenseErrors', () => { From 21e5e05dad5e33627a0f6b9dbbcd38e8d7785679 Mon Sep 17 00:00:00 2001 From: Abhishek Date: Tue, 21 Oct 2025 21:49:05 +0530 Subject: [PATCH 07/11] [Remove Vuetify from Studio] Buttons/links in Import from channels --- .../ImportFromChannelsModal.vue | 36 +++++++++---------- .../SearchOrBrowseWindow.vue | 25 +++++++------ .../ImportFromChannels/SearchResultsList.vue | 3 +- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ImportFromChannelsModal.vue b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ImportFromChannelsModal.vue index 004cc44828..98847971e7 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ImportFromChannelsModal.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/ImportFromChannelsModal.vue @@ -52,21 +52,19 @@ > check_circle {{ $tr('addedText') }} - - {{ $tr('removeButton') }} - + /> - - {{ $tr('addButton') }} - + /> diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue index ceb73ca731..373fe19ee7 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/ImportFromChannels/SearchOrBrowseWindow.vue @@ -13,8 +13,9 @@ v-if="!isBrowsing" class="my-2" > - @@ -44,25 +45,27 @@ -
- + - From 008ca8342db70d128853ee7c1ad33134581c8296 Mon Sep 17 00:00:00 2001 From: Blaine Jester Date: Tue, 21 Oct 2025 10:52:29 -0700 Subject: [PATCH 08/11] Push div wrapper outside of VLayout, to restore epub renderer height --- .../channelEdit/views/files/ContentRenderer.vue | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/contentcuration/contentcuration/frontend/channelEdit/views/files/ContentRenderer.vue b/contentcuration/contentcuration/frontend/channelEdit/views/files/ContentRenderer.vue index b363de29b6..96d578c135 100644 --- a/contentcuration/contentcuration/frontend/channelEdit/views/files/ContentRenderer.vue +++ b/contentcuration/contentcuration/frontend/channelEdit/views/files/ContentRenderer.vue @@ -1,12 +1,12 @@ From db480b83c2e652a4942c781aac787175389a1202 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Fri, 24 Oct 2025 15:17:27 +0300 Subject: [PATCH 09/11] distingushes recommendation and recommendation events endpoints --- .../frontend/shared/feedbackApiUtils.js | 6 +-- .../tests/viewsets/test_recommendations.py | 38 ++++++++++++------- contentcuration/contentcuration/urls.py | 10 +++-- 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/feedbackApiUtils.js b/contentcuration/contentcuration/frontend/shared/feedbackApiUtils.js index 49ba10d165..bdc8f15879 100644 --- a/contentcuration/contentcuration/frontend/shared/feedbackApiUtils.js +++ b/contentcuration/contentcuration/frontend/shared/feedbackApiUtils.js @@ -12,9 +12,9 @@ export const FeedbackTypeOptions = { flagged: 'FLAGGED', }; -export const FLAG_FEEDBACK_EVENT_ENDPOINT = 'flagged'; -export const RECOMMENDATION_EVENT_ENDPOINT = 'recommendations'; -export const RECOMMENDATION_INTERACTION_EVENT_ENDPOINT = 'recommendations-interaction'; +export const FLAG_FEEDBACK_EVENT_ENDPOINT = 'flagged-events'; +export const RECOMMENDATION_EVENT_ENDPOINT = 'recommendations-events'; +export const RECOMMENDATION_INTERACTION_EVENT_ENDPOINT = 'recommendations-interaction-events'; /** * @typedef {Object} BaseFeedbackParams diff --git a/contentcuration/contentcuration/tests/viewsets/test_recommendations.py b/contentcuration/contentcuration/tests/viewsets/test_recommendations.py index e792cfc75b..dcc865244f 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_recommendations.py +++ b/contentcuration/contentcuration/tests/viewsets/test_recommendations.py @@ -186,14 +186,16 @@ def setUp(self): def test_create_recommendations_event(self): recommendations_event = self.recommendations_event_object response = self.client.post( - reverse("recommendations-list"), + reverse("recommendations-events-list"), recommendations_event, format="json", ) self.assertEqual(response.status_code, 201, response.content) def test_list_fails(self): - response = self.client.get(reverse("recommendations-list"), format="json") + response = self.client.get( + reverse("recommendations-events-list"), format="json" + ) self.assertEqual(response.status_code, 405, response.content) def test_retrieve_fails(self): @@ -214,7 +216,9 @@ def test_retrieve_fails(self): user=self.user, ) response = self.client.get( - reverse("recommendations-detail", kwargs={"pk": recommendations_event.id}), + reverse( + "recommendations-events-detail", kwargs={"pk": recommendations_event.id} + ), format="json", ) self.assertEqual(response.status_code, 405, response.content) @@ -242,7 +246,9 @@ def test_update_recommendations_event(self): "breadcrumbs": "#Title#->Updated", } response = self.client.put( - reverse("recommendations-detail", kwargs={"pk": recommendations_event.id}), + reverse( + "recommendations-events-detail", kwargs={"pk": recommendations_event.id} + ), updated_data, format="json", ) @@ -266,7 +272,9 @@ def test_partial_update_recommendations_event(self): user=self.user, ) response = self.client.patch( - reverse("recommendations-detail", kwargs={"pk": recommendations_event.id}), + reverse( + "recommendations-events-detail", kwargs={"pk": recommendations_event.id} + ), {"context": {"model_version": 2}}, format="json", ) @@ -290,7 +298,9 @@ def test_destroy_recommendations_event(self): user=self.user, ) response = self.client.delete( - reverse("recommendations-detail", kwargs={"pk": recommendations_event.id}), + reverse( + "recommendations-events-detail", kwargs={"pk": recommendations_event.id} + ), format="json", ) self.assertEqual(response.status_code, 405, response.content) @@ -345,7 +355,7 @@ def setUp(self): def test_create_recommendations_interaction(self): recommendations_interaction = self.recommendations_interaction_object response = self.client.post( - reverse("recommendations-interaction-list"), + reverse("recommendations-interaction-events-list"), recommendations_interaction, format="json", ) @@ -371,7 +381,7 @@ def test_bulk_create_recommendations_interaction(self): }, ] response = self.client.post( - reverse("recommendations-interaction-list"), + reverse("recommendations-interaction-events-list"), recommendations_interactions, format="json", ) @@ -399,7 +409,7 @@ def test_bulk_create_recommendations_interaction_failure(self): }, ] response = self.client.post( - reverse("recommendations-interaction-list"), + reverse("recommendations-interaction-events-list"), recommendations_interactions, format="json", ) @@ -408,7 +418,7 @@ def test_bulk_create_recommendations_interaction_failure(self): def test_list_fails(self): response = self.client.get( - reverse("recommendations-interaction-list"), format="json" + reverse("recommendations-interaction-events-list"), format="json" ) self.assertEqual(response.status_code, 405, response.content) @@ -423,7 +433,7 @@ def test_retrieve_fails(self): ) response = self.client.get( reverse( - "recommendations-interaction-detail", + "recommendations-interaction-events-detail", kwargs={"pk": recommendations_interaction.id}, ), format="json", @@ -443,7 +453,7 @@ def test_update_recommendations_interaction(self): updated_data["feedback_type"] = "PREVIEWED" response = self.client.put( reverse( - "recommendations-interaction-detail", + "recommendations-interaction-events-detail", kwargs={"pk": recommendations_interaction.id}, ), updated_data, @@ -462,7 +472,7 @@ def test_partial_update_recommendations_interaction(self): ) response = self.client.patch( reverse( - "recommendations-interaction-detail", + "recommendations-interaction-events-detail", kwargs={"pk": recommendations_interaction.id}, ), {"feedback_type": "IMPORTED"}, @@ -481,7 +491,7 @@ def test_destroy_recommendations_interaction(self): ) response = self.client.delete( reverse( - "recommendations-interaction-detail", + "recommendations-interaction-events-detail", kwargs={"pk": recommendations_interaction.id}, ), format="json", diff --git a/contentcuration/contentcuration/urls.py b/contentcuration/contentcuration/urls.py index 59cf09e891..720bbc3d55 100644 --- a/contentcuration/contentcuration/urls.py +++ b/contentcuration/contentcuration/urls.py @@ -76,14 +76,16 @@ def get_redirect_url(self, *args, **kwargs): router.register(r"assessmentitem", AssessmentItemViewSet) router.register(r"admin-users", AdminUserViewSet, basename="admin-users") router.register(r"clipboard", ClipboardViewSet, basename="clipboard") -router.register(r"flagged", FlagFeedbackEventViewSet, basename="flagged") +router.register(r"events/flagged", FlagFeedbackEventViewSet, basename="flagged-events") router.register( - r"recommendations", RecommendationsEventViewSet, basename="recommendations" + r"events/recommendations", + RecommendationsEventViewSet, + basename="recommendations-events", ) router.register( - r"recommendationsinteraction", + r"events/recommendationsinteraction", RecommendationsInteractionEventViewSet, - basename="recommendations-interaction", + basename="recommendations-interaction-events", ) urlpatterns = [ From b17e9ee67fb1df06e77abe880b53ffc8bb312584 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Fri, 24 Oct 2025 16:59:44 +0300 Subject: [PATCH 10/11] Fixes failing tests --- .../tests/viewsets/test_flagged.py | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/contentcuration/contentcuration/tests/viewsets/test_flagged.py b/contentcuration/contentcuration/tests/viewsets/test_flagged.py index 1f2acf3ac2..f82b1c4b73 100644 --- a/contentcuration/contentcuration/tests/viewsets/test_flagged.py +++ b/contentcuration/contentcuration/tests/viewsets/test_flagged.py @@ -34,7 +34,7 @@ def test_create_flag_event(self): self.client.force_authenticate(user=self.user) flagged_content = self.flag_feedback_object response = self.client.post( - reverse("flagged-list"), + reverse("flagged-events-list"), flagged_content, format="json", ) @@ -46,7 +46,7 @@ def test_create_flag_event_fails_for_flag_test_dev_feature_disabled(self): self.user.save() self.client.force_authenticate(user=self.user) response = self.client.post( - reverse("flagged-list"), + reverse("flagged-events-list"), flagged_content, format="json", ) @@ -58,7 +58,7 @@ def test_create_flag_event_fails_for_flag_test_dev_feature_None(self): self.user.save() self.client.force_authenticate(user=self.user) response = self.client.post( - reverse("flagged-list"), + reverse("flagged-events-list"), flagged_content, format="json", ) @@ -67,7 +67,7 @@ def test_create_flag_event_fails_for_flag_test_dev_feature_None(self): def test_create_flag_event_fails_for_unauthorized_user(self): flagged_content = self.flag_feedback_object response = self.client.post( - reverse("flagged-list"), + reverse("flagged-events-list"), flagged_content, format="json", ) @@ -77,7 +77,7 @@ def test_list_flagged_content_super_admin(self): self.user.is_admin = True self.user.save() self.client.force_authenticate(self.user) - response = self.client.get(reverse("flagged-list"), format="json") + response = self.client.get(reverse("flagged-events-list"), format="json") self.assertEqual(response.status_code, 200, response.content) def test_retreive_fails_for_normal_user(self): @@ -94,18 +94,18 @@ def test_retreive_fails_for_normal_user(self): user=self.user, ) response = self.client.get( - reverse("flagged-detail", kwargs={"pk": flag_feedback_object.id}), + reverse("flagged-events-detail", kwargs={"pk": flag_feedback_object.id}), format="json", ) self.assertEqual(response.status_code, 403, response.content) def test_list_fails_for_normal_user(self): self.client.force_authenticate(user=self.user) - response = self.client.get(reverse("flagged-list"), format="json") + response = self.client.get(reverse("flagged-events-list"), format="json") self.assertEqual(response.status_code, 403, response.content) def test_list_fails_for_user_dev_feature_enabled(self): - response = self.client.get(reverse("flagged-list"), format="json") + response = self.client.get(reverse("flagged-events-list"), format="json") self.assertEqual(response.status_code, 403, response.content) def test_destroy_flagged_content_super_admin(self): @@ -124,7 +124,7 @@ def test_destroy_flagged_content_super_admin(self): user=self.user, ) response = self.client.delete( - reverse("flagged-detail", kwargs={"pk": flag_feedback_object.id}), + reverse("flagged-events-detail", kwargs={"pk": flag_feedback_object.id}), format="json", ) self.assertEqual(response.status_code, 204, response.content) @@ -145,7 +145,7 @@ def test_destroy_flagged_content_fails_for_user_with_feature_flag_disabled(self) user=self.user, ) response = self.client.delete( - reverse("flagged-detail", kwargs={"pk": flag_feedback_object.id}), + reverse("flagged-events-detail", kwargs={"pk": flag_feedback_object.id}), format="json", ) self.assertEqual(response.status_code, 403, response.content) From 8d5df6c9654c67b3233c0ae1ce3b06f2e32fb7a4 Mon Sep 17 00:00:00 2001 From: Richard Tibbles Date: Mon, 27 Oct 2025 07:22:16 -0700 Subject: [PATCH 11/11] Add file duration setting as migration task --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 002d337323..a4160f6b99 100644 --- a/Makefile +++ b/Makefile @@ -38,7 +38,7 @@ migrate: # 4) Remove the management command from this `deploy-migrate` recipe # 5) Repeat! deploy-migrate: - echo "Nothing to do here!" + python contentcuration/manage.py set_file_duration contentnodegc: python contentcuration/manage.py garbage_collect