Skip to content

Commit 0c54634

Browse files
authored
Merge pull request #3290 from NationalSecurityAgency/3.5.X-merge3
3.5.x merge3
2 parents 10ecc8a + 69d3914 commit 0c54634

File tree

5 files changed

+112
-58
lines changed

5 files changed

+112
-58
lines changed

dashboard/src/common-components/utilities/UseCheckIfAnswerChangedForValidation.js

+27-18
Original file line numberDiff line numberDiff line change
@@ -13,33 +13,42 @@
1313
* See the License for the specific language governing permissions and
1414
* limitations under the License.
1515
*/
16+
import {useLog} from "@/components/utils/misc/useLog.js";
17+
1618
export const useCheckIfAnswerChangedForValidation = () => {
1719
const cache = new Map()
18-
const hasValueChanged = (newValue, testContext) => {
19-
const quizAnswers = testContext?.parent.quizAnswers
20-
if (quizAnswers && quizAnswers.length === 1 && quizAnswers[0].id) {
21-
const answerId = quizAnswers[0].id
22-
const cachedValue = cache.get(answerId)
23-
if (cachedValue === newValue) {
24-
return false
20+
const log = useLog()
21+
const getStatusIfValueTheSame = (answerId, newValue) => {
22+
if (!answerId) {
23+
return null
24+
}
25+
const cachedItem = cache.get(answerId)
26+
if (cachedItem) {
27+
const isValueSame = cachedItem.value === newValue
28+
if (log.isTraceEnabled()) {
29+
log.trace(`useCheckIfAnswerChangedForValidation.hasValueChanged(newValue=${newValue}, answerId=${answerId}): isValueSame=${isValueSame}, cachedItem=${JSON.stringify(cachedItem)}`)
30+
}
31+
if (isValueSame) {
32+
return cachedItem.result
2533
}
26-
cache.set(answerId, newValue)
2734
}
28-
return true
35+
return null
36+
}
37+
38+
const setValueAndStatus = (answerId, value, result) => {
39+
if (log.isTraceEnabled()) {
40+
log.trace(`useCheckIfAnswerChangedForValidation.setValueAndStatus(answerId=${answerId}, value=${value}, result=${JSON.stringify(result)})`)
41+
}
42+
cache.set(answerId, { value, result })
2943
}
44+
3045
const reset = () => {
46+
log.trace(`useCheckIfAnswerChangedForValidation.reset()`)
3147
cache.clear()
3248
}
33-
const removeAnswer = (testContext) => {
34-
const quizAnswers = testContext?.parent.quizAnswers
35-
if (quizAnswers && quizAnswers.length === 1 && quizAnswers[0].id) {
36-
const answerId = quizAnswers[0].id
37-
cache.delete(answerId)
38-
}
39-
}
4049
return {
41-
hasValueChanged,
42-
removeAnswer,
50+
getStatusIfValueTheSame,
51+
setValueAndStatus,
4352
reset
4453
}
4554
}

dashboard/src/skills-display/components/quiz/QuizRun.vue

+54-11
Original file line numberDiff line numberDiff line change
@@ -91,27 +91,42 @@ const getQuestionNumFromPath = (path) => {
9191
}
9292
9393
const validateFunCache = new Map()
94+
95+
const getAnswerIdFromContext = (testContext) => {
96+
const quizAnswers = testContext?.parent.quizAnswers
97+
if (quizAnswers && quizAnswers.length === 1 && quizAnswers[0].id) {
98+
return quizAnswers[0].id
99+
}
100+
return null
101+
}
94102
const createValidateAnswerFn = (valueOuter, contextOuter) => {
95103
if (!QuestionType.isTextInput(contextOuter.parent.questionType)) {
96104
return true
97105
}
106+
const getResponseBasedOnResult = (result, resContext) => {
107+
if (result.valid) {
108+
return true
109+
}
110+
if (result.msg) {
111+
return resContext.createError({ message: `Answer to question #${getQuestionNumFromPath(resContext.path)} - ${result.msg}` })
112+
}
113+
return resContext.createError({ message: `'Field' is invalid` })
114+
}
98115
const doValidateAnswer = (value, context) => {
99116
if (!value || value.trim().length === 0 || !appConfig.paragraphValidationRegex) {
100117
return true
101118
}
102119
const forceAnswerValidation = isSubmitting.value
103-
if (!forceAnswerValidation && !checkIfAnswerChangedForValidation.hasValueChanged(context.originalValue, context)) {
104-
return true
120+
if (!forceAnswerValidation) {
121+
const existingResultIfValueTheSame = checkIfAnswerChangedForValidation.getStatusIfValueTheSame(getAnswerIdFromContext(context), context.originalValue)
122+
if (existingResultIfValueTheSame !== null) {
123+
return getResponseBasedOnResult(existingResultIfValueTheSame, context)
124+
}
105125
}
126+
106127
return descriptionValidatorService.validateDescription(value, false, null, true).then((result) => {
107-
if (result.valid) {
108-
return true
109-
}
110-
checkIfAnswerChangedForValidation.removeAnswer(context)
111-
if (result.msg) {
112-
return context.createError({ message: `Answer to question #${getQuestionNumFromPath(context.path)} - ${result.msg}` })
113-
}
114-
return context.createError({ message: `'Field' is invalid` })
128+
checkIfAnswerChangedForValidation.setValueAndStatus(getAnswerIdFromContext(context), value, result)
129+
return getResponseBasedOnResult(result, context)
115130
})
116131
}
117132
@@ -133,6 +148,7 @@ const schema = object({
133148
.of(
134149
object({
135150
'questionType': string(),
151+
// important: please note that this logic has to match what's performed by `validateTextAnswer` method beow
136152
'answerText': string()
137153
.trim()
138154
.max(appConfig.maxTakeQuizInputTextAnswerLength, (d) => `Answer to question #${getQuestionNumFromPath(d.path)} must not exceed ${numFormat.pretty(appConfig.maxTakeQuizInputTextAnswerLength)} characters`)
@@ -168,6 +184,33 @@ const { values, meta, handleSubmit, isSubmitting, resetForm, setFieldValue, vali
168184
validationSchema: schema,
169185
})
170186
187+
// important: please note that this logic has to match what's performed by `answerText` in the yup schema above
188+
const validateTextAnswer = (value) => {
189+
validateField(value.fieldName)
190+
const textAnswer = value.answerText?.trim()
191+
if (textAnswer && textAnswer.length === 0) {
192+
return Promise.resolve(false);
193+
}
194+
if (textAnswer && textAnswer.length > appConfig.maxTakeQuizInputTextAnswerLength) {
195+
return Promise.resolve(false);
196+
}
197+
198+
if (!appConfig.paragraphValidationRegex) {
199+
return Promise.resolve(true)
200+
}
201+
const existingStatusIfValueTheSame = checkIfAnswerChangedForValidation.getStatusIfValueTheSame(value.answerId, value.answerText)
202+
if (existingStatusIfValueTheSame !== null) {
203+
return Promise.resolve(existingStatusIfValueTheSame.valid)
204+
}
205+
return descriptionValidatorService.validateDescription(value.answerText, false, null, true).then((result) => {
206+
checkIfAnswerChangedForValidation.setValueAndStatus(value.answerId, value.answerText, result)
207+
if (result.valid) {
208+
return true
209+
}
210+
return false
211+
})
212+
}
213+
171214
onMounted(() => {
172215
if (props.quiz) {
173216
setQuizInfo(({ ...props.quiz }));
@@ -477,7 +520,7 @@ const doneWithThisRun = () => {
477520
:quiz-id="quizId"
478521
:quiz-attempt-id="quizAttemptId"
479522
:num="index+1"
480-
:validate="validateField"
523+
:validate="validateTextAnswer"
481524
@selected-answer="updateSelectedAnswers"
482525
@answer-text-changed="updateSelectedAnswers"/>
483526
</div>

dashboard/src/skills-display/components/quiz/QuizRunQuestion.vue

+5-4
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ const textAnswerChanged = (providedAnswerText) => {
117117
});
118118
});
119119
}
120-
const textAnswerChangedDebounced = useDebounceFn((providedAnswerTextOuter) => textAnswerChanged(providedAnswerTextOuter), appConfig.formFieldDebounceInMs, appConfig.formFieldDebounceInMs)
120+
const textAnswerChangedDebounced = useDebounceFn((providedAnswerTextOuter) => textAnswerChanged(providedAnswerTextOuter), appConfig.formFieldDebounceInMs)
121121
122122
const selectionChanged = (currentAnswer) => {
123123
reportAnswer(currentAnswer).then((reportAnswerPromise) => {
@@ -149,9 +149,10 @@ const ratingChanged = (value) => {
149149
const reportAnswer = (answer) => {
150150
if (!isLoading.value) {
151151
const reportAnswer = () => QuizRunService.reportAnswer(props.quizId, props.quizAttemptId, answer.changedAnswerId, answer.changedAnswerIdSelected, answer.answerText)
152-
if (props.validate && QuestionType.isTextInput(props.q.questionType)) {
153-
return props.validate(fieldName.value).then((validationResults) => {
154-
if (validationResults.valid) {
152+
if (QuestionType.isTextInput(props.q.questionType) ) {
153+
const answerInfo = { answerText: answer.answerText, fieldName: fieldName.value, quizId: props.quizId, quizAttemptId: props.quizAttemptId, answerId: answer.changedAnswerId };
154+
return props.validate(answerInfo).then((isValid) => {
155+
if (isValid) {
155156
return reportAnswer()
156157
}
157158
return null;

e2e-tests/cypress/e2e/quiz/run_quiz_with_text_input_spec.js

+24-23
Original file line numberDiff line numberDiff line change
@@ -391,14 +391,15 @@ describe('Run Quizzes With Text Input Questions', () => {
391391
cy.get('[data-cy="startQuizAttempt"]').click()
392392
cy.get('[data-cy="question_1"] [data-cy="markdownEditorInput"]').type('Answer to question #1')
393393
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]').type('Answer to question #2 jabberwoc')
394-
cy.wait(2000)
394+
cy.wait(3000)
395395
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]').type('ky')
396396
cy.get('[data-cy="question_2"] [data-cy="descriptionError"]').contains('Answer to question #2 - paragraphs may not contain jabberwocky')
397397

398398
cy.wait('@validateDescriptionAnswer1')
399399
cy.wait('@validateDescriptionAnswer2')
400400
cy.get('@validateDescriptionAnswer1.all').should('have.length', 1)
401-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 1)
401+
// can be 1 or two depending on order of execution
402+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lt', 3)
402403

403404
cy.get('[data-cy="question_1"] [data-cy="markdownEditorInput"]').type('X')
404405
cy.get('@validateDescriptionAnswer1.all').should('have.length', 1)
@@ -437,7 +438,7 @@ describe('Run Quizzes With Text Input Questions', () => {
437438

438439
// validation called once when user typed answer, and again on submit
439440
cy.get('@validateDescriptionAnswer1.all').should('have.length', 2)
440-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 2)
441+
cy.get('@validateDescriptionAnswer2.all').should('have.length', 1)
441442
});
442443

443444
it('Input Text validation: answer cache is reset when starting a quiz (after refresh), and validation endpoint called for all questions', () => {
@@ -475,27 +476,27 @@ describe('Run Quizzes With Text Input Questions', () => {
475476
cy.wait(1000)
476477
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]').type('Z')
477478
cy.wait(1000)
478-
cy.get('@validateDescriptionAnswer1.all').should('have.length', 1)
479-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 4)
479+
cy.get('@validateDescriptionAnswer1.all').should('have.length.lte', 2)
480+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lte', 5)
480481

481482
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]').type('Z')
482-
cy.get('@validateDescriptionAnswer1.all').should('have.length', 1)
483-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 5)
483+
cy.get('@validateDescriptionAnswer1.all').should('have.length.lte', 2)
484+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lte', 6)
484485

485486
// reload the page, all answers are revalidated on load (or visit?)
486487
cy.visit('/progress-and-rankings/quizzes/quiz1');
487488
cy.get('[data-cy="subPageHeader"]').contains('Quiz')
488489
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]')
489490
cy.wait('@validateDescriptionAnswer1')
490491
cy.wait('@validateDescriptionAnswer2')
491-
cy.get('@validateDescriptionAnswer1.all').should('have.length', 2)
492-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 6)
492+
cy.get('@validateDescriptionAnswer1.all').should('have.length.lte', 3)
493+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lte', 7)
493494

494495
// update answer 2 and only answer 2 gets revalidated
495496
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]').type('Z')
496497
cy.wait('@validateDescriptionAnswer2')
497-
cy.get('@validateDescriptionAnswer1.all').should('have.length', 2)
498-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 7)
498+
cy.get('@validateDescriptionAnswer1.all').should('have.length.lte', 3)
499+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lte', 8)
499500
});
500501

501502
it('Input Text validation: answer cache is reset when starting a quiz (after navigating away), and validation endpoint called for all questions', () => {
@@ -537,12 +538,12 @@ describe('Run Quizzes With Text Input Questions', () => {
537538
cy.wait(1000)
538539
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]').type('Z')
539540
cy.wait(1000)
540-
cy.get('@validateDescriptionAnswer1.all').should('have.length', 1)
541-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 4)
541+
cy.get('@validateDescriptionAnswer1.all').should('have.length.lte', 2)
542+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lte', 5)
542543

543544
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]').type('Z')
544-
cy.get('@validateDescriptionAnswer1.all').should('have.length', 1)
545-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 5)
545+
cy.get('@validateDescriptionAnswer1.all').should('have.length.lte', 2)
546+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lte', 6)
546547

547548
cy.visit('/progress-and-rankings/quizzes/quiz2');
548549
cy.get('[data-cy="subPageHeader"]').contains('Quiz')
@@ -560,28 +561,28 @@ describe('Run Quizzes With Text Input Questions', () => {
560561
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]').type('Answer to question #2')
561562

562563
// one more for each question
563-
cy.get('@validateDescriptionAnswer1.all').should('have.length', 2)
564-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 6)
564+
cy.get('@validateDescriptionAnswer1.all').should('have.length.lte', 3)
565+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lte', 7)
565566

566567
// update answer 2 and only answer 2 gets revalidated
567568
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]').type('Z')
568-
cy.get('@validateDescriptionAnswer1.all').should('have.length', 2)
569-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 7)
569+
cy.get('@validateDescriptionAnswer1.all').should('have.length.lte', 3)
570+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lte', 8)
570571

571572
// navigate back to quiz 1, all answers are revalidated on load
572573
cy.visit('/progress-and-rankings/quizzes/quiz1');
573574
cy.get('[data-cy="subPageHeader"]').contains('Quiz')
574575
cy.wait('@validateDescriptionAnswer1')
575576
cy.wait('@validateDescriptionAnswer2')
576577
cy.wait(1000)
577-
cy.get('@validateDescriptionAnswer1.all').should('have.length', 3)
578-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 8)
578+
cy.get('@validateDescriptionAnswer1.all').should('have.length.lte', 4)
579+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lte', 8)
579580

580581
// update answer 2 and only answer 2 gets revalidated
581582
cy.get('[data-cy="question_2"] [data-cy="markdownEditorInput"]').type('Z')
582583
cy.wait('@validateDescriptionAnswer2')
583-
cy.get('@validateDescriptionAnswer1.all').should('have.length', 3)
584-
cy.get('@validateDescriptionAnswer2.all').should('have.length', 9)
584+
cy.get('@validateDescriptionAnswer1.all').should('have.length.lte', 4)
585+
cy.get('@validateDescriptionAnswer2.all').should('have.length.lte', 9)
585586
});
586587

587588
});

pom.xml

+2-2
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
<guava.version>33.4.6-jre</guava.version>
3535

3636
<!-- !!!!!IMPORTANT!!!!!: when changing the springboot.version property, make sure you also change it in the spring-boot-starter-parent definition -->
37-
<springboot.version>3.4.4</springboot.version>
37+
<springboot.version>3.4.5</springboot.version>
3838
<springcloud.version>3.1.1</springcloud.version>
3939

4040
<logback-access.version>2.0.6</logback-access.version>
@@ -60,7 +60,7 @@
6060
<groupId>org.springframework.boot</groupId>
6161
<artifactId>spring-boot-starter-parent</artifactId>
6262
<!-- !!!!!IMPORTANT!!!!!: when changing this version make sure to also update springboot.version property -->
63-
<version>3.4.4</version>
63+
<version>3.4.5</version>
6464
<relativePath/>
6565
</parent>
6666
<repositories>

0 commit comments

Comments
 (0)