diff --git a/core/migrations/0018_auto_20190814_1324.py b/core/migrations/0018_auto_20190814_1324.py new file mode 100644 index 0000000000000000000000000000000000000000..68d141b6b8dffc9442e9a1e04b5c433872e3990f --- /dev/null +++ b/core/migrations/0018_auto_20190814_1324.py @@ -0,0 +1,18 @@ +# Generated by Django 2.1.11 on 2019-08-14 13:24 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0017_auto_20190604_1631'), + ] + + operations = [ + migrations.AlterField( + model_name='feedbacklabel', + name='name', + field=models.CharField(max_length=50, unique=True), + ), + ] diff --git a/core/migrations/0019_merge_20190814_1437.py b/core/migrations/0019_merge_20190814_1437.py new file mode 100644 index 0000000000000000000000000000000000000000..be3acd875f07d9910f23a48e771324e0d1b25e51 --- /dev/null +++ b/core/migrations/0019_merge_20190814_1437.py @@ -0,0 +1,14 @@ +# Generated by Django 2.1.11 on 2019-08-14 14:37 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0018_auto_20190709_1526'), + ('core', '0018_auto_20190814_1324'), + ] + + operations = [ + ] diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py index da0d45e5501cbd03f39b2101cd65344bcb4da80f..fbbc9b32dd8c749f4ecaa7aa4a4d6dd7c411279a 100644 --- a/core/serializers/feedback.py +++ b/core/serializers/feedback.py @@ -64,6 +64,8 @@ class FeedbackCommentDictionarySerializer(serializers.ListSerializer): class FeedbackCommentSerializer(DynamicFieldsModelSerializer): of_tutor = serializers.StringRelatedField(source='of_tutor.username') + labels = serializers.PrimaryKeyRelatedField(many=True, required=False, + queryset=models.FeedbackLabel.objects.all()) class Meta: model = models.FeedbackComment @@ -151,13 +153,15 @@ class FeedbackSerializer(DynamicFieldsModelSerializer): feedback.final_by_reviewer = self.context['request'].data['is_final'] for comment in validated_data.pop('feedback_lines', []): - labels = comment.pop('labels', []) + labels = comment.pop('labels', None) comment_instance, _ = models.FeedbackComment.objects.update_or_create( of_feedback=feedback, of_tutor=self.context['request'].user, of_line=comment.get('of_line'), defaults={'text': comment.get('text')}) - comment_instance.labels.set(labels) + + if labels is not None: + comment_instance.labels.set(labels) return super().update(feedback, validated_data) diff --git a/frontend/src/api.ts b/frontend/src/api.ts index 1fa6700d78eb3fe7818911fd61fd5746032b8087..d6a1d8381075625c645d20f1d453911f39f80c72 100644 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -12,7 +12,8 @@ import { SubmissionNoType, SubmissionType, Subscription, Tutor, UserAccount, LabelStatisticsForSubType, - FeedbackLabel + FeedbackLabel, + CreateUpdateFeedback } from '@/models' function getInstanceBaseUrl (): string { @@ -151,11 +152,13 @@ export async function createAssignment ( return (await ax.post(`/api/assignment/`, data)).data } -export async function submitFeedbackForAssignment ({ feedback }: {feedback: Partial<Feedback>}): Promise<Feedback> { +export async function submitFeedbackForAssignment ({ feedback }: + { feedback: Partial<CreateUpdateFeedback>}): Promise<CreateUpdateFeedback> { return (await ax.post('/api/feedback/', feedback)).data } -export async function submitUpdatedFeedback ({ feedback }: {feedback: Feedback}): Promise<Feedback> { +export async function submitUpdatedFeedback ({ feedback }: + {feedback: CreateUpdateFeedback}): Promise<CreateUpdateFeedback> { return (await ax.patch(`/api/feedback/${feedback.ofSubmission}/`, feedback)).data } diff --git a/frontend/src/components/mixins/commentLabelSelector.ts b/frontend/src/components/mixins/commentLabelSelector.ts index 4076ac3dcf485638581b8ecd7694bf0d7bfb74ce..4351b1b5a5a85c735b50c5d3c542e2c84894b02c 100644 --- a/frontend/src/components/mixins/commentLabelSelector.ts +++ b/frontend/src/components/mixins/commentLabelSelector.ts @@ -5,6 +5,11 @@ import { SubmissionNotes } from "@/store/modules/submission-notes" import { FeedbackComment, FeedbackLabel } from "@/models" import { FeedbackLabels } from "@/store/modules/feedback-labels" +enum FeedbackType { + original = "origFeedback", + updated = "updatedFeedback", +} + @Component export default class commentLabelSelector extends Vue { @Prop({ type: String, required: true }) readonly lineNo!: string @@ -12,35 +17,51 @@ export default class commentLabelSelector extends Vue { /** * Returns array of label pk's where feedbackType is * either "origFeedback" or "updatedFeedback" + * + * Will return null when labels property does not exist on the requested state's comment + * This is the case when the labels have not been updated, as we don't want to have + * the labels field in the object if the labels have not changed. */ - copyStateLabels(feedbackType: string): number[] { - if (feedbackType !== "origFeedback" && feedbackType !== "updatedFeedback") return new Array() - + copyStateLabels(feedbackType: FeedbackType): number[] | null { const currentLine = this.getFeedbackLine(feedbackType) - return currentLine ? currentLine.labels : new Array() + if (currentLine && currentLine.labels) { + return currentLine.labels + } else { + return null + } } - getFeedbackLine (feedbackType: string): FeedbackComment |Â undefined { - if (feedbackType !== "origFeedback" && feedbackType !== "updatedFeedback") return undefined + /** + * Gets the latest feedback line object for the current lineNo and the given feedbackType + * @param feedbackType The type to get the latest line from + */ + getFeedbackLine (feedbackType: FeedbackType): FeedbackComment |Â undefined { + + // helper used to determine the correct type to reduce redundancy + function isArray(val: FeedbackComment | FeedbackComment[]): val is FeedbackComment[] { + return (<FeedbackComment[]>val).length !== undefined + } const stateLines = SubmissionNotes.state[feedbackType].feedbackLines if (stateLines && Object.keys(stateLines).length > 0) { let lines = stateLines[Number(this.lineNo)] - if (lines === undefined) return undefined + if (!lines) return undefined - // always copy latest comment - if (lines.length > 0) { - return lines[lines.length-1] + if (isArray(lines)) { + return lines.length > 0 ? lines[lines.length-1] : undefined } else { - // @ts-ignore return lines } } + + return undefined } getUnchangedLabels() { - const labelsOrig = this.copyStateLabels("origFeedback") - if (labelsOrig === undefined) return new Array() + const labelsOrig = this.copyStateLabels(FeedbackType.original) + if (labelsOrig === null || labelsOrig.length === 0) { + return new Array () + } const removedLabels = this.getRemovedLabels() const addedLabels = this.getAddedLabels() @@ -51,13 +72,15 @@ export default class commentLabelSelector extends Vue { } getRemovedLabels() { - const currentLine = this.getFeedbackLine("updatedFeedback") + const currentLine = this.getFeedbackLine(FeedbackType.updated) if (currentLine === undefined) return new Array() - const labelsOrig = this.copyStateLabels("origFeedback") - const labelsUpdated = this.copyStateLabels("updatedFeedback") + const labelsOrig = this.copyStateLabels(FeedbackType.original) + const labelsUpdated = this.copyStateLabels(FeedbackType.updated) - if (labelsOrig == undefined) return new Array() + if (labelsOrig === null || labelsUpdated === null) { + return new Array() + } return labelsOrig.filter((val) => { return !labelsUpdated.includes(val) @@ -65,10 +88,16 @@ export default class commentLabelSelector extends Vue { } getAddedLabels() { - const labelsOrig = this.copyStateLabels("origFeedback") - const labelsUpdated = this.copyStateLabels("updatedFeedback") + const labelsOrig = this.copyStateLabels(FeedbackType.original) + const labelsUpdated = this.copyStateLabels(FeedbackType.updated) - if (labelsOrig === undefined) return new Array() + if (labelsUpdated === null) { + return new Array() + } + + if (labelsOrig === null) { + return labelsUpdated ? labelsUpdated : new Array() + } return labelsUpdated.filter((val) => { return !labelsOrig.includes(val) @@ -84,15 +113,22 @@ export default class commentLabelSelector extends Vue { return label.pk === val }) - if (!label) return + if (!label) return undefined return { pk: val, name: label.name, description: label.description, colour: label.colour } + }).filter((val): val is FeedbackLabel => { + if (!val) { + console.error("Encountered invalid label pk. This should be investigated.") + return false + } + + return true }) - return mappedLabels ? mappedLabels : new Array() + return mappedLabels; } } \ No newline at end of file diff --git a/frontend/src/components/submission_notes/base/CommentForm.vue b/frontend/src/components/submission_notes/base/CommentForm.vue index 83481e557fd5803e74e858fb0be675400ea4d15e..4e512500cc6927398a5a002990b4ddc78e3d7332 100644 --- a/frontend/src/components/submission_notes/base/CommentForm.vue +++ b/frontend/src/components/submission_notes/base/CommentForm.vue @@ -104,13 +104,19 @@ export default class CommentForm extends mixins(commentLabelSelector) { } submitFeedback () { - SubmissionNotes.UPDATE_FEEDBACK_LINE({ + const payload = { lineNo: Number(this.lineNo), comment: { text: this.currentFeedback, labels: this.labelsUnchanged.concat(this.labelsAdded), } - }) + } + + if (this.labelsAdded.length === 0 && this.labelsRemoved.length === 0) { + delete payload.comment.labels + } + + SubmissionNotes.UPDATE_FEEDBACK_LINE(payload) this.collapseTextField() } } diff --git a/frontend/src/components/submission_notes/base/FeedbackComment.vue b/frontend/src/components/submission_notes/base/FeedbackComment.vue index 963fe4b4c42cc4abe9d798779edb471a84c853f2..1b9a1c063e0959273cfe3c0fab8272d69cbe8226 100644 --- a/frontend/src/components/submission_notes/base/FeedbackComment.vue +++ b/frontend/src/components/submission_notes/base/FeedbackComment.vue @@ -148,22 +148,6 @@ export default { backgroundColor () { return UI.state.darkMode ? 'grey' : '#F3F3F3' }, - labelsToShow () { - const mappedLabels = this.labels.map((val) => { - const label = Labels.availableLabels.find((label) => { - return label.pk === val - }) - - if (!label) return new Array() - return { - pk: val, - name: label.name, - description: label.description, - colour: label.colour - } - }) - return mappedLabels ? mappedLabels : new Array() - }, unchangedLabels() { return this.mapPksToLabelObj(this.getUnchangedLabels()) }, diff --git a/frontend/src/models.ts b/frontend/src/models.ts index 4065aa3273dfda5f1c288a64ee568a2bc803689b..ffde4332511307816cf681382c4cf4216ef141f9 100644 --- a/frontend/src/models.ts +++ b/frontend/src/models.ts @@ -142,6 +142,63 @@ export interface Feedback { labels: number[], } +/** + * + * @export + * @interface CreateUpdateFeedback + */ +export interface CreateUpdateFeedback { + /** + * + * @type {number} + * @memberof Feedback + */ + pk: number + /** + * + * @type {string} + * @memberof Feedback + */ + ofSubmission?: string + /** + * + * @type {boolean} + * @memberof Feedback + */ + isFinal?: boolean + /** + * + * @type {number} + * @memberof Feedback + */ + score?: number + /** + * + * @type {Array<FeedbackComment>} + * @memberof Feedback + */ + feedbackLines?: {[lineNo: number]: FeedbackComment} + /** + * + * @type {Date} + * @memberof Feedback + */ + created?: string + /** + * + * @type {string} + * @memberof Feedback + */ + ofSubmissionType?: string + /** + * + * @type {string} + * @memberof Feedback + */ + feedbackStageForUser?: string, + labels: number[], +} + /** * * @export diff --git a/frontend/src/store/modules/submission-notes.ts b/frontend/src/store/modules/submission-notes.ts index 9055ea8149c89903f09f1973c44e527e3e4440b4..8247114b9ad17c68863caa3f4937175b4c862e68 100644 --- a/frontend/src/store/modules/submission-notes.ts +++ b/frontend/src/store/modules/submission-notes.ts @@ -1,7 +1,7 @@ import Vue from 'vue' import * as hljs from 'highlight.js' import * as api from '@/api' -import { Feedback, FeedbackComment, SubmissionNoType, FeedbackLabel } from '@/models' +import { Feedback, FeedbackComment, SubmissionNoType, CreateUpdateFeedback } from '@/models' import { RootState } from '@/store/store' import { getStoreBuilder, BareActionContext } from 'vuex-typex' import { syntaxPostProcess } from '@/util/helpers'; @@ -16,7 +16,7 @@ export interface SubmissionNotesState { }, hasOrigFeedback: boolean origFeedback: Feedback - updatedFeedback: Feedback + updatedFeedback: CreateUpdateFeedback commentsMarkedForDeletion: { [pk: string]: FeedbackComment } changedLabels: boolean } @@ -163,11 +163,18 @@ async function submitFeedback( { isFinal = false}): Promise<AxiosResponse<void>[]> { - let feedback: Partial<Feedback> = { + let feedback: Partial<CreateUpdateFeedback> = { isFinal: isFinal, ofSubmission: state.submission.pk, + feedbackLines: {}, labels: state.updatedFeedback.labels } + + // omit labels for the request + if (!state.changedLabels) { + delete feedback.labels + } + if (state.origFeedback.score === undefined && state.updatedFeedback.score === undefined) { throw new Error('You need to give a score.') } else if (state.updatedFeedback.score !== undefined) { @@ -176,8 +183,14 @@ Promise<AxiosResponse<void>[]> { feedback.score = state.origFeedback.score } - if (Object.keys(state.updatedFeedback.feedbackLines || {}).length > 0) { - feedback.feedbackLines = state.updatedFeedback.feedbackLines + if (state.updatedFeedback.feedbackLines && Object.keys(state.updatedFeedback.feedbackLines).length > 0) { + // set the comments for the feedback lines accordingly + for (const key of Object.keys(state.updatedFeedback.feedbackLines)) { + const numKey = Number(key) + + numKey && feedback.feedbackLines + && (feedback.feedbackLines[numKey] = state.updatedFeedback.feedbackLines[numKey]) + } } else if (feedback.score! < SubmissionNotes.submissionType.fullScore! && !state.hasOrigFeedback) { throw new Error('You need to add or change a comment when setting a non full score.') } @@ -185,7 +198,7 @@ Promise<AxiosResponse<void>[]> { await api.submitFeedbackForAssignment({ feedback }) } else { feedback.pk = state.origFeedback.pk - await api.submitUpdatedFeedback(<{ feedback: Feedback }>{ feedback }) + await api.submitUpdatedFeedback(<{ feedback: CreateUpdateFeedback }>{ feedback }) } // delete those comments that have been marked for deletion return SubmissionNotes.deleteComments()