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

Move handleReplacement logic from ReplaceQuestions.vue to useQuizCreation.js #12099

Merged
merged 1 commit into from
Apr 23, 2024
Merged

Move handleReplacement logic from ReplaceQuestions.vue to useQuizCreation.js #12099

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 21 additions & 0 deletions kolibri/plugins/coach/assets/src/composables/useQuizCreation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ export default function useQuizCreation() {
/** @type {ref<Number>} A counter for use in naming new sections */
const _sectionLabelCounter = ref(1);

/** @type {ref<Array>} A list of all Question objects selected for replacement */
const replacements = ref([]);

// ------------------
// Section Management
// ------------------
Expand Down Expand Up @@ -163,6 +166,16 @@ export default function useQuizCreation() {
});
}

function handleReplacement() {
const questionsNotSelectedToBeReplaced = activeQuestions.value.filter(
question => !selectedActiveQuestions.value.includes(question.id)
);
updateSection({
section_id: activeSection.value.section_id,
questions: [...questionsNotSelectedToBeReplaced, ...replacements.value],
});
}

/**
* @description Selects random questions from the active section's `resource_pool` - no side
* effects
Expand Down Expand Up @@ -481,6 +494,7 @@ export default function useQuizCreation() {

provide('saveQuiz', saveQuiz);
provide('updateSection', updateSection);
provide('handleReplacement', handleReplacement);
provide('replaceSelectedQuestions', replaceSelectedQuestions);
provide('addSection', addSection);
provide('removeSection', removeSection);
Expand All @@ -491,6 +505,7 @@ export default function useQuizCreation() {
provide('removeQuestionFromSelection', removeQuestionFromSelection);
provide('clearSelectedQuestions', clearSelectedQuestions);
provide('channels', channels);
provide('replacements', replacements);
provide('quiz', quiz);
provide('allSections', allSections);
provide('activeSection', activeSection);
Expand All @@ -511,6 +526,7 @@ export default function useQuizCreation() {
// Methods
saveQuiz,
updateSection,
handleReplacement,
replaceSelectedQuestions,
addSection,
removeSection,
Expand All @@ -523,6 +539,7 @@ export default function useQuizCreation() {

// Computed
channels,
replacements,
quiz,
allSections,
activeSection,
Expand Down Expand Up @@ -552,6 +569,7 @@ export default function useQuizCreation() {
export function injectQuizCreation() {
const saveQuiz = inject('saveQuiz');
const updateSection = inject('updateSection');
const handleReplacement = inject('handleReplacement');
const replaceSelectedQuestions = inject('replaceSelectedQuestions');
const addSection = inject('addSection');
const removeSection = inject('removeSection');
Expand All @@ -562,6 +580,7 @@ export function injectQuizCreation() {
const removeQuestionFromSelection = inject('removeQuestionFromSelection');
const clearSelectedQuestions = inject('clearSelectedQuestions');
const channels = inject('channels');
const replacements = inject('replacements');
const quiz = inject('quiz');
const allSections = inject('allSections');
const activeSection = inject('activeSection');
Expand All @@ -584,6 +603,7 @@ export function injectQuizCreation() {
deleteActiveSelectedQuestions,
selectAllQuestions,
updateSection,
handleReplacement,
replaceSelectedQuestions,
addSection,
removeSection,
Expand All @@ -599,6 +619,7 @@ export function injectQuizCreation() {
allQuestionsSelected,
selectAllIsIndeterminate,
channels,
replacements,
quiz,
allSections,
activeSection,
Expand Down
21 changes: 7 additions & 14 deletions kolibri/plugins/coach/assets/src/views/plan/CreateExamPage/ReplaceQuestions.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@
:cancelText="coreString('cancelAction')"
:title="replaceQuestions$()"
@cancel="showReplacementConfirmation = false"
@submit="handleReplacement"
@submit="submitReplacement"
>
<div> {{ replaceQuestionsExplaination$() }} </div>
<div style="font-weight: bold;">
Expand Down Expand Up @@ -188,7 +188,6 @@
} = enhancedQuizManagementStrings;
const {
// Computed
activeQuestions,
activeSection,
selectedActiveQuestions,
activeResourceMap,
Expand All @@ -199,6 +198,8 @@
replaceSelectedQuestions,
toggleQuestionInSelection,
updateSection,
handleReplacement,
replacements,
} = injectQuizCreation();

const showCloseConfirmation = ref(false);
Expand All @@ -208,17 +209,9 @@
context.emit('closePanel');
}

const replacements = ref([]);

function handleReplacement() {
function submitReplacement() {
handleReplacement();
Comment on lines +212 to +213
Copy link
Member

Choose a reason for hiding this comment

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

Is there any particular reason to call this from within a function rather than returning handleReplacement from the setup function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe we needed the router logic to be placed inside the ReplaceQuestions file itself. Returning handleReplacement from the setup function and directly calling it from @submit handler would require the routing logic to be placed in the handleReplacement function that is now moved to useQuizCreation.
That was the main reason for me to call it from within a function as that provides separation of concerns.

Though I'd like to know if I'm missing something here and if there's a better workaround for this.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh thanks for that - I misread the diff 😅 - I thought the only thing happening in submitReplacement was calling handleReplacement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh 😅. No problem :)

const count = replacements.value.length;
const questionsNotSelectedToBeReplaced = activeQuestions.value.filter(
question => !selectedActiveQuestions.value.includes(question.id)
);
updateSection({
section_id: activeSection.value.section_id,
questions: [...questionsNotSelectedToBeReplaced, ...replacements.value],
});
router.replace({
name: PageNames.EXAM_CREATION_ROOT,
query: {
Expand Down Expand Up @@ -263,8 +256,6 @@

return {
toggleInReplacements,
handleReplacement,
replacements,
activeSection,
selectAllReplacementQuestions,
selectedActiveQuestions,
Expand All @@ -283,6 +274,8 @@
isItemExpanded,
toggleQuestionInSelection,
updateSection,
submitReplacement,
replacements,
replaceQuestions$,
deleteSectionLabel$,
replaceAction$,
Expand Down