diff --git a/app/assets/javascripts/components/input_table.ts b/app/assets/javascripts/components/input_table.ts new file mode 100644 index 0000000000..13fdbaccf2 --- /dev/null +++ b/app/assets/javascripts/components/input_table.ts @@ -0,0 +1,333 @@ +import { customElement, property } from "lit/decorators.js"; +import { html, PropertyValues, render, TemplateResult } from "lit"; +import jspreadsheet, { CellValue, Column, CustomEditor, JspreadsheetInstance } from "jspreadsheet-ce"; +import { createRef, ref, Ref } from "lit/directives/ref.js"; +import { DodonaElement } from "components/meta/dodona_element"; +import { fetch, ready } from "utilities"; +import { i18n } from "i18n/i18n"; +import { Tooltip } from "bootstrap"; +import { watchMixin } from "components/meta/watch_mixin"; + +type CellData = string | number | boolean; +type ScoreItem = { + id: number | null; + name: string; + description?: string; + maximum: string; + visible: boolean; + order?: number; +} + +type ColumnWithTooltip = Column & { tooltip?: string }; + +const toBoolean = (value: CellValue): boolean => { + return value === "true" || value === true; +}; + +/** + * A spreadsheet table to edit score items. + * + * @element d-score-item-input-table + * + * @fires cancel - When the cancel button is clicked. + * + * @prop {string} route - The route to send the updated score items to. + * @prop {ScoreItem[]} scoreItems - The original score items, that will be displayed in the table. + */ +@customElement("d-score-item-input-table") +export class ScoreItemInputTable extends watchMixin(DodonaElement) { + @property({ type: String }) + route: string = ""; + @property({ type: Array, attribute: "score-items" }) + scoreItems: ScoreItem[] = []; + @property({ type: Boolean, attribute: "total-visible" }) + totalVisible: boolean = false; + + tableRef: Ref = createRef(); + table: JspreadsheetInstance; + + @property({ state: true }) + hasErrors: boolean = false; + @property({ state: true }) + _totalVisible: boolean = false; + + watch = { + totalVisible: () => { + this._totalVisible = this.totalVisible; + } + }; + + toggleTotalVisible(): void { + this._totalVisible = !this._totalVisible; + } + + get tableWidth(): number { + return this.tableRef.value.clientWidth; + } + + get descriptionColWidth(): number { + if (!this.tableRef.value) { + return 200; + } + + // full width - borders - name column - maximum column - visible column - index column - delete column + const variableWidth = this.tableWidth - 14 - 200 - 75 - 75 - 50 - 30; + return Math.max(200, variableWidth); + } + + get data(): CellData[][] { + return [ + ...this.scoreItems.map(item => [ + item.id, + item.name, + item.description, + item.maximum, + item.visible + ]), + ["", "", "", "", false] + ]; + } + + get editedScoreItems(): ScoreItem[] { + const tableData = this.table.getData(); + + const scoreItems = tableData.map((row: CellData[], index: number) => { + return { + id: row[0] as number | null, + name: row[1] as string, + description: row[2] as string, + maximum: (row[3] as string).replace(",", "."), // replace comma with dot for float representation + visible: toBoolean(row[4]), + order: index, + }; + }); + + // filter out empty rows + return scoreItems.filter(item => !(item.name === "" && item.maximum === "" && item.description === "" && item.visible === false)); + } + + deleteCellRow(cell: HTMLTableCellElement): void { + const row = cell.parentElement as HTMLTableRowElement; + this.table.deleteRow(row.rowIndex-1); + } + + createDeleteButton(cell: HTMLTableCellElement): HTMLTableCellElement { + const button = html``; + render(button, cell); + return cell; + } + + customCheckboxEditor(): CustomEditor { + const updateCell = (cell: HTMLTableCellElement): void => { + this.table.setValue(cell, !toBoolean(this.table.getValue(cell))); + }; + return { + createCell: (cell: HTMLTableCellElement) => { + const current = cell.innerHTML === "true"; + const checkbox = html`
+ +
`; + cell.innerHTML = ""; + render(checkbox, cell); + return cell; + }, + openEditor: () => false, + closeEditor: (cell: HTMLTableCellElement) => { + return toBoolean(this.table.getValue(cell)); + }, + updateCell: (cell: HTMLTableCellElement, value: CellValue) => { + const checkbox = cell.querySelector("input"); + if (checkbox) { + checkbox.checked = toBoolean(value); + } + return toBoolean(value); + } + }; + } + + get columnConfig(): ColumnWithTooltip[] { + return [ + { type: "hidden", title: "id" }, + { type: "text", title: i18n.t("js.score_items.name"), width: 200, align: "left" }, + { type: "text", title: i18n.t("js.score_items.description"), width: this.descriptionColWidth, align: "left", tooltip: i18n.t("js.score_items.description_help") }, + { type: "numeric", title: i18n.t("js.score_items.maximum"), width: 75, align: "left", tooltip: i18n.t("js.score_items.maximum_help") }, + { type: "html", title: i18n.t("js.score_items.visible"), width: 75, align: "left", tooltip: i18n.t("js.score_items.visible_help"), editor: this.customCheckboxEditor() }, + { type: "html", title: " ", width: 30, align: "center", readOnly: true, editor: { + createCell: (cell: HTMLTableCellElement) => this.createDeleteButton(cell), + } }, + ]; + } + + async initTable(): Promise { + // Wait for translations to be present + await ready; + + this.table = jspreadsheet(this.tableRef.value, { + root: this, + data: this.data, + columns: this.columnConfig, + text: { + copy: i18n.t("js.score_items.jspreadsheet.copy"), + deleteSelectedRows: i18n.t("js.score_items.jspreadsheet.deleteSelectedRows"), + insertANewRowAfter: i18n.t("js.score_items.jspreadsheet.insertNewRowAfter"), + insertANewRowBefore: i18n.t("js.score_items.jspreadsheet.insertNewRowBefore"), + paste: i18n.t("js.score_items.jspreadsheet.paste"), + }, + about: false, + allowDeleteColumn: false, + allowDeleteRow: true, + allowInsertColumn: false, + allowInsertRow: true, + allowManualInsertColumn: false, + allowManualInsertRow: true, + allowRenameColumn: false, + columnResize: false, + columnSorting: false, + minSpareRows: 1, + parseFormulas: false, + selectionCopy: false, + wordWrap: true, + defaultRowHeight: 30, + allowExport: false, + }); + + // init tooltips + this.columnConfig.forEach((column, index) => { + const td = this.tableRef.value.querySelector(`thead td[data-x="${index}"]`); + if (td && column.tooltip) { + td.setAttribute("title", column.tooltip); + new Tooltip(td); + } + }); + + // mark header and menu as non-editable + this.tableRef.value.querySelector("thead").setAttribute("contenteditable", "false"); + this.tableRef.value.querySelector(".jexcel_contextmenu").setAttribute("contenteditable", "false"); + + + // update description column width when the window is resized + new ResizeObserver(() => { + this.table.setWidth(2, this.descriptionColWidth); + }).observe(this.tableRef.value); + } + + protected firstUpdated(_changedProperties: PropertyValues): void { + super.firstUpdated(_changedProperties); + + this.initTable(); + } + + validate(): boolean { + // Remove all error classes + this.tableRef.value.querySelectorAll("td.error").forEach(cell => { + cell.classList.remove("error"); + }); + + const invalidCells: string[] = []; + const data = this.editedScoreItems; + data.forEach(item => { + const row = item.order + 1; + if (item.name === "") { + invalidCells.push("B" + row); + } + // Check if maximum is a positive number < 1000 + // we use a regex instead of parseFloat because parseFloat is too lenient + if (!/^\d{1,3}(\.\d+)?$/.test(item.maximum) || parseFloat(item.maximum) <= 0) { + invalidCells.push("D" + row); + } + }); + invalidCells.forEach(cell => { + this.table.getCell(cell).classList.add("error"); + }); + this.hasErrors = invalidCells.length > 0; + return !this.hasErrors; + } + + confirmWarnings(): boolean { + const old = this.scoreItems; + const edited = this.editedScoreItems; + const removed = old.some(item => !edited.some(e => e.id === item.id)); + const maxEdited = old.some(item => edited.some(e => e.id === item.id && e.maximum !== item.maximum)); + + let warnings = ""; + if (removed) { + warnings += i18n.t("js.score_items.deleted_warning") + "\n"; + } + if (maxEdited) { + warnings += i18n.t("js.score_items.modified_warning") + "\n"; + } + + return warnings === "" || confirm(warnings); + } + + async save(): Promise { + if (!this.validate()) { + return; + } + + if (!this.confirmWarnings()) { + return; + } + + const response = await fetch(this.route, { + method: "PATCH", + headers: { + "Content-Type": "application/json" + }, + body: JSON.stringify({ + evaluation_exercise: { + visible_score: this._totalVisible, + score_items: this.editedScoreItems + } + }) + }); + if (response.ok) { + const js = await response.text(); + eval(js); + } + } + + cancel(): void { + if (this.table) { + this.table.setData(this.data); + this._totalVisible = this.totalVisible; + this.hasErrors = false; + } + this.dispatchEvent(new Event("cancel")); + } + + + render(): TemplateResult { + return html` + ${this.hasErrors ? html` +
${i18n.t("js.score_items.validation_warning")}
` : ""} +
+
+ + this.toggleTotalVisible()}> +
+
+ + +
+ `; + } +} diff --git a/app/assets/javascripts/i18n/translations.json b/app/assets/javascripts/i18n/translations.json index fa881ec177..f6f81c7301 100644 --- a/app/assets/javascripts/i18n/translations.json +++ b/app/assets/javascripts/i18n/translations.json @@ -330,6 +330,29 @@ "score_item": { "error": "Error while updating" }, + "score_items": { + "cancel": "Cancel", + "deleted_warning": "You have deleted one or more score items. This will also delete all scores for these items.", + "description": "Description", + "description_help": "A description is optional. Markdown formatting can be used. This is visible to the students.", + "jspreadsheet": { + "copy": "Copy...", + "deleteRow": "Delete row", + "deleteSelectedRows": "Delete selected rows", + "insertNewRowAfter": "Insert new row after", + "insertNewRowBefore": "Insert new row before", + "paste": "Paste..." + }, + "maximum": "Maximum", + "maximum_help": "The maximum grade for this score item. The grade should be between 0 and 1000, and works in increments of 0.25.", + "modified_warning": "You have changed the maximum score of one or more score items. This will mark all completed evaluations with this score item as uncompleted.", + "name": "Name", + "save": "Save", + "total_visible": "Make the total score visible to students once the evaluation is released.", + "validation_warning": "All score items must have a name and a maximum score, and the maximum score must be between 0 and 1000.", + "visible": "Visible", + "visible_help": "Make the score item visible to students once the evaluation is released." + }, "search": { "filter": { "course_id": "Course", @@ -825,6 +848,29 @@ "score_item": { "error": "Fout bij bijwerken" }, + "score_items": { + "cancel": "Annuleren", + "deleted_warning": "Je hebt een of meerdere scoreonderdelen verwijderd. Dit zal ook de bijhorende scores van de studenten verwijderen.", + "description": "Beschrijving", + "description_help": "Een beschrijving is optioneel en kan in Markdown geschreven worden. Dit is zichtbaar voor de studenten.", + "jspreadsheet": { + "copy": "Kopieer...", + "deleteRow": "Verwijder rij", + "deleteSelectedRows": "Verwijder geselecteerde rijen", + "insertNewRowAfter": "Voeg nieuwe rij toe na deze", + "insertNewRowBefore": "Voeg nieuwe rij toe voor deze", + "paste": "Plak..." + }, + "maximum": "Maximum", + "maximum_help": "De maximumscore voor dit scoreonderdeel. Dit moet een getal zijn tussen 0 en 1000 en gaat in stappen van 0.25.", + "modified_warning": "Je hebt de maximumscore van een of meerdere scoreonderdelen aangepast. Alle afgewerkte evaluaties met dit scoreonderdeel zullen terug als onafgewerkt gemarkeerd worden.", + "name": "Naam", + "save": "Opslaan", + "total_visible": "Maak totaal score zichtbaar voor studenten eens de evaluatie vrijgegeven is.", + "validation_warning": "Alle scoreonderdelen moeten een naam en een maximumscore hebben. De maximumscore moet een getal zijn tussen 0 en 1000.", + "visible": "Zichtbaar", + "visible_help": "Maak het scoreonderdeel zichtbaar voor studenten eens de evaluatie vrijgegeven is." + }, "search": { "filter": { "course_id": "Cursus", diff --git a/app/assets/javascripts/score_item.ts b/app/assets/javascripts/score_item.ts index 5a5b331f08..dbbf8bbf79 100644 --- a/app/assets/javascripts/score_item.ts +++ b/app/assets/javascripts/score_item.ts @@ -1,5 +1,6 @@ import { fetch } from "utilities"; import { i18n } from "i18n/i18n"; +import { ScoreItemInputTable } from "components/input_table"; function commonCheckboxInit( element: HTMLElement, @@ -44,18 +45,21 @@ function initTotalVisibilityCheckboxes(element: HTMLElement): void { }); } -function initItemVisibilityCheckboxes(element: HTMLElement): void { - commonCheckboxInit(element, ".visibility-checkbox", checked => { - return { +export function initVisibilityCheckboxes(element: HTMLElement): void { + initTotalVisibilityCheckboxes(element); +} - score_item: { - visible: checked - } - }; +export function initEditButton(element: HTMLElement): void { + const table = element.querySelector(".score-items-table") as HTMLTableElement; + const form = element.querySelector("d-score-item-input-table") as ScoreItemInputTable; + + table.addEventListener("click", () => { + table.classList.add("d-none"); + form.classList.remove("d-none"); }); -} -export function initVisibilityCheckboxes(element: HTMLElement): void { - initTotalVisibilityCheckboxes(element); - initItemVisibilityCheckboxes(element); + form.addEventListener("cancel", () => { + table.classList.remove("d-none"); + form.classList.add("d-none"); + }); } diff --git a/app/assets/javascripts/utilities.ts b/app/assets/javascripts/utilities.ts index 7e2f1c2f22..45416c65cd 100644 --- a/app/assets/javascripts/utilities.ts +++ b/app/assets/javascripts/utilities.ts @@ -269,5 +269,5 @@ export { ready, getParentByClassName, setHTMLExecuteScripts, - replaceHTMLExecuteScripts, + replaceHTMLExecuteScripts }; diff --git a/app/assets/stylesheets/base.css.scss b/app/assets/stylesheets/base.css.scss index 796425716e..36e4c23586 100644 --- a/app/assets/stylesheets/base.css.scss +++ b/app/assets/stylesheets/base.css.scss @@ -64,11 +64,16 @@ @import "../../../node_modules/bootstrap/scss/offcanvas"; @import "../../../node_modules/bootstrap/scss/utilities/api"; +// jspreadsheet styles +@import "../../../node_modules/jspreadsheet-ce/dist/jspreadsheet"; +@import "../../../node_modules/jsuites/dist/jsuites"; + :root { --scrollbar-width: 20px; } // 5. Add additional custom code here +@import "theme/jspreadsheet.css.scss"; @import "bootstrap_style_overrides.css.scss"; @import "libraries.css.scss"; @import "material_icons.css.scss"; diff --git a/app/assets/stylesheets/components/btn.css.scss b/app/assets/stylesheets/components/btn.css.scss index 03d843b89b..7559bba1d3 100644 --- a/app/assets/stylesheets/components/btn.css.scss +++ b/app/assets/stylesheets/components/btn.css.scss @@ -207,6 +207,20 @@ @include shadow-z2; } } + + &.btn-icon-inline { + padding: 0; + width: auto; + height: auto; + color: var(--d-text-muted); + + &:hover, + &:focus, + &:active { + background: none; + color: var(--d-btn-color); + } + } } .btn.btn-fab, diff --git a/app/assets/stylesheets/models/score_items.css.scss b/app/assets/stylesheets/models/score_items.css.scss index b4638df288..efee2a8df1 100644 --- a/app/assets/stylesheets/models/score_items.css.scss +++ b/app/assets/stylesheets/models/score_items.css.scss @@ -5,6 +5,15 @@ } .score-items-table { + cursor: text; + + tr:not(:last-child) { + td:hover { + background: rgba(var(--d-primary-rgb), 0.1); + } + } + + td.description { max-width: 150px; @@ -14,11 +23,11 @@ } td.visibility { - width: 20px; + width: 60px; } td.maximum { - width: 105px; + width: 100px; padding-right: 20px; } @@ -31,8 +40,13 @@ width: 250px; } - td.wide-description { - width: 650px; + td.add-score-items { + text-align: center; + + // padding is used to center the button + // padding-right: 90px = 250px (td.name) - 60px (td.visibility) - 100px (td.maximum) + // top and bottom padding is 0 to avoid double padding with the button + padding: 0 90px 0 0; } .maximum-row { @@ -88,3 +102,13 @@ a.option-btn { } } } + +d-score-item-input-table .jexcel tr:last-child td:last-child button { + opacity: 0.38; + pointer-events: none; + + &:focus { + opacity: 0.38; + pointer-events: none; + } +} diff --git a/app/assets/stylesheets/theme/jspreadsheet.css.scss b/app/assets/stylesheets/theme/jspreadsheet.css.scss new file mode 100644 index 0000000000..937e55aa75 --- /dev/null +++ b/app/assets/stylesheets/theme/jspreadsheet.css.scss @@ -0,0 +1,213 @@ +// Overwrites colors in node_modules/jspreadsheet-ce/dist/jspreadsheet.css + +:root { + --jexcel-border-color: var(--d-on-surface); +} + +/* stylelint-disable-next-line selector-class-pattern */ +.jexcel_container.fullscreen .jexcel_content { + background-color: var(--d-surface); +} + +/* stylelint-disable-next-line selector-class-pattern */ +.jexcel_content::-webkit-scrollbar-track { + background: var(--d-background); +} + + +.jexcel { + background-color: var(--d-surface); + border-top: 1px solid transparent; + border-left: 1px solid transparent; + border-right: 1px solid var(--d-divider); + border-bottom: 1px solid var(--d-divider); +} + +.jexcel td::selection { + background-color: transparent; +} + +.jexcel > thead > tr > td { + border-top: 1px solid var(--d-divider); + border-left: 1px solid var(--d-divider); + border-right: 1px solid transparent; + border-bottom: 1px solid transparent; + background-color: var(--d-code-bg); + font-weight: 700; + line-height: 16px; + letter-spacing: 0; + font-size: 12px; +} + +.jexcel > thead > tr > td.dragging { + background-color: var(--d-surface); +} + +.jexcel > thead > tr > td.selected { + background-color: rgba(var(--d-on-surface-rgb), 0.25); +} + +.jexcel > tbody > tr > td:first-child { + background-color: var(--d-code-bg); + font-weight: 700; + line-height: 16px; + letter-spacing: 0; + font-size: 12px; +} + +.jexcel > tbody > tr.dragging > td { + background-color: var(--d-background); +} + +.jexcel > tbody > tr > td { + border-top: 1px solid var(--d-divider); + border-left: 1px solid var(--d-divider); + border-right: 1px solid transparent; + border-bottom: 1px solid transparent; +} + +.jexcel > tbody > tr > td.readonly { + color: rgba(var(--d-on-surface-rgb), 0.3); +} + +.jexcel > tbody > tr.selected > td:first-child { + background-color: rgba(var(--d-on-surface-rgb), 0.25); +} + +.jexcel > tbody > tr > td.highlight > a { + color: var(--d-info); +} + +.jexcel > tfoot > tr > td { + border-top: 1px solid var(--d-divider); + border-left: 1px solid var(--d-divider); + border-right: 1px solid transparent; + border-bottom: 1px solid transparent; + background-color: var(--d-code-bg); +} + +.jexcel .highlight { + background-color: rgba(var(--d-on-surface-rgb), 0.05); +} + +.jexcel .highlight-top { + border-top: 1px solid var(--jexcel-border-color); + box-shadow: 0 -1px var(--d-divider); +} + +.jexcel .highlight-left { + border-left: 1px solid var(--jexcel-border-color); + box-shadow: -1px 0 var(--d-divider); +} + +.jexcel .highlight-right { + border-right: 1px solid var(--jexcel-border-color); +} + +.jexcel .highlight-bottom { + border-bottom: 1px solid var(--jexcel-border-color); +} + +.jexcel .highlight-top.highlight-left { + box-shadow: -1px -1px var(--d-divider); +} + +.jexcel .highlight-selected { + background-color: rgba(var(--d-on-surface-rgb), 0.0); +} + +.jexcel .selection { + background-color: rgba(var(--d-on-surface-rgb), 0.05); +} + +.jexcel .selection-left { + border-left: 1px dotted var(--jexcel-border-color); +} + +.jexcel .selection-right { + border-right: 1px dotted var(--jexcel-border-color); +} + +.jexcel .selection-top { + border-top: 1px dotted var(--jexcel-border-color); +} + +.jexcel .selection-bottom { + border-bottom: 1px dotted var(--jexcel-border-color); +} + +.jexcel .editor .jupload { + background-color: var(--d-surface); +} + +/* stylelint-disable-next-line selector-class-pattern */ +.jexcel .editor .jexcel_richtext { + background-color: var(--d-surface); +} + +.jexcel .editor .jclose::after { + text-shadow: 0 0 5px var(--d-surface); +} + +.jexcel .error { + border: 1px solid var(--d-danger); +} + +.jexcel .copying-top { + border-top: 1px dashed var(--jexcel-border-color); +} + +.jexcel .copying-left { + border-left: 1px dashed var(--jexcel-border-color); +} + +.jexcel .copying-right { + border-right: 1px dashed var(--jexcel-border-color); +} + +.jexcel .copying-bottom { + border-bottom: 1px dashed var(--jexcel-border-color); +} + +.red { + color: var(--d-danger); +} + +// overwrites colors and pading for jcontextmenu defined in node_modules/jsuites/dist/jsuites.css + +.jcontextmenu { + background: var(--d-surface); + color: var(--d-on-surface); + border: 1px solid var(--d-divider); + border-radius: 5px; + padding: 0; + + @include shadow-z3; +} + +.jcontextmenu > div { + padding: 8px 16px; + width: auto; +} + +.jcontextmenu > div a { + color: var(--d-on-surface); +} + +.jcontextmenu .jcontextmenu-disabled a { + color: var(--d-on-surface-muted); +} + +.jcontextmenu .jcontextmenu-disabled::before { + color: var(--d-on-surface-muted); +} + +.jcontextmenu > div:hover { + background: var(--d-secondary-container); + color: var(--d-on-secondary-container); +} + +.jcontextmenu hr { + border: 1px solid var(--d-divider); + margin: 0; +} diff --git a/app/controllers/evaluation_exercise_controller.rb b/app/controllers/evaluation_exercise_controller.rb index f63fcb8a29..fe91e220ea 100644 --- a/app/controllers/evaluation_exercise_controller.rb +++ b/app/controllers/evaluation_exercise_controller.rb @@ -3,8 +3,31 @@ class EvaluationExerciseController < ApplicationController def update @evaluation_exercise.update!(permitted_attributes(@evaluation_exercise)) + + if params[:evaluation_exercise].key?(:score_items) + score_items = params[:evaluation_exercise][:score_items] + ScoreItem.transaction do + new_items = score_items.filter { |item| !item.key?(:id) || item[:id].blank? } + updated_items = score_items.filter { |item| item[:id].present? } + @evaluation_exercise.score_items.each do |item| + if (updated_item = updated_items.find { |i| i[:id].to_i == item.id }) + item.update!(updated_item.permit(:name, :description, :maximum, :visible, :order)) + else + item.destroy + end + end + new_items.each do |item| + @evaluation_exercise.score_items.create!(item.permit(:name, :description, :maximum, :visible, :order)) + end + rescue ActiveRecord::RecordInvalid => e + return render json: { errors: e.record.errors }, status: :unprocessable_entity + end + end + respond_to do |format| - format.js { render 'score_items/index', locals: { new: nil, evaluation_exercise: @evaluation_exercise } } + @evaluation.reload + format.js { render 'score_items/index', locals: { new: nil, evaluation_exercise: @evaluation_exercise.reload } } + format.json { head :no_content } end end diff --git a/app/controllers/evaluations_controller.rb b/app/controllers/evaluations_controller.rb index baeb149580..4a092c2849 100644 --- a/app/controllers/evaluations_controller.rb +++ b/app/controllers/evaluations_controller.rb @@ -127,7 +127,7 @@ def update def modify_grading_visibility new_visibility = ActiveModel::Type::Boolean.new.cast(params[:visible]) @evaluation.change_grade_visibility!(new_visibility) - redirect_back fallback_location: evaluation_score_items_path(@evaluation) + redirect_back fallback_location: edit_evaluation_path(@evaluation) end def set_multi_user diff --git a/app/controllers/score_items_controller.rb b/app/controllers/score_items_controller.rb deleted file mode 100644 index 0b67381489..0000000000 --- a/app/controllers/score_items_controller.rb +++ /dev/null @@ -1,98 +0,0 @@ -class ScoreItemsController < ApplicationController - include SeriesHelper - - before_action :set_score_item, only: %i[destroy update] - before_action :set_evaluation - - def copy - from = EvaluationExercise.find(params[:copy][:from]) - to = EvaluationExercise.find(params[:copy][:to]) - - from.score_items.each do |score_item| - new_score_item = score_item.dup - new_score_item.evaluation_exercise = to - new_score_item.last_updated_by = current_user - new_score_item.save - end - - # Score items have changed. - @evaluation.score_items.reload - respond_to do |format| - format.js { render 'score_items/index', locals: { new: nil, evaluation_exercise: to } } - end - end - - def create - @score_item = ScoreItem.new(permitted_attributes(ScoreItem)) - @score_item.last_updated_by = current_user - respond_to do |format| - if @score_item.save - format.js { render 'score_items/index', locals: { new: nil, evaluation_exercise: preload_eval_exercise(@score_item) } } - format.json { render :show, status: :created, location: [@evaluation, @score_item] } - else - format.js { render 'score_items/index', locals: { new: @score_item, evaluation_exercise: preload_eval_exercise(@score_item) } } - format.json { render json: @score_item.errors, status: :unprocessable_entity } - end - end - end - - def update - args = permitted_attributes(@score_item) - args[:last_updated_by] = current_user - respond_to do |format| - if @score_item.update(args) - format.js { render 'score_items/index', locals: { new: nil, evaluation_exercise: preload_eval_exercise(@score_item) } } - format.json { render :show, status: :ok, location: [@evaluation, @score_item] } - else - format.js { render 'score_items/index', locals: { new: @score_item, evaluation_exercise: preload_eval_exercise(@score_item) } } - format.json { render json: @score_item.errors, status: :unprocessable_entity } - end - end - end - - def add_all - @score_item = ScoreItem.new(permitted_attributes(ScoreItem, :create)) - @score_item.last_updated_by = current_user - # Add all score items or none. - @evaluation.transaction do - @evaluation.evaluation_exercises.each do |evaluation_exercise| - new_score_item = @score_item.dup - evaluation_exercise.score_items << new_score_item - end - end - @evaluation.reload - end - - def destroy - @score_item.destroy - respond_to do |format| - format.js { render 'score_items/index', locals: { new: nil, evaluation_exercise: preload_eval_exercise(@score_item) } } - format.json { head :no_content } - end - end - - private - - def set_score_item - @score_item = ScoreItem.find(params[:id]) - end - - def set_evaluation - # Include a bunch of stuff to reduce number of queries on show pages. - @evaluation = Evaluation.includes(evaluation_exercises: :exercise, score_items: :scores).find(params[:evaluation_id]) - authorize @evaluation, :score_items? - - @crumbs = [ - [@evaluation.series.course.name, course_path(@evaluation.series.course)], - [@evaluation.series.name, breadcrumb_series_path(@evaluation.series, current_user)], - [I18n.t('evaluations.show.evaluation'), evaluation_path(@evaluation)] - ] - end - - def preload_eval_exercise(item) - # Reload the evaluation, since one of the items changed. - @evaluation.score_items.reload - # Preload the stuff for one exercise and an existing instance. - EvaluationExercise.includes({ score_items: :scores }).find(item.evaluation_exercise.id) - end -end diff --git a/app/javascript/packs/score_item.js b/app/javascript/packs/score_item.js index 79aeb544fc..310fc8fd32 100644 --- a/app/javascript/packs/score_item.js +++ b/app/javascript/packs/score_item.js @@ -1,3 +1,5 @@ -import { initVisibilityCheckboxes } from "score_item.ts"; +import { initVisibilityCheckboxes, initEditButton } from "score_item.ts"; +import "components/input_table.ts"; window.dodona.initVisibilityCheckboxes = initVisibilityCheckboxes; +window.dodona.initEditButton = initEditButton; diff --git a/app/models/score_item.rb b/app/models/score_item.rb index b7031b95da..2e4b8ecedb 100644 --- a/app/models/score_item.rb +++ b/app/models/score_item.rb @@ -10,6 +10,7 @@ # description :text(65535) # created_at :datetime not null # updated_at :datetime not null +# order :integer # class ScoreItem < ApplicationRecord belongs_to :evaluation_exercise @@ -23,7 +24,7 @@ class ScoreItem < ApplicationRecord validates :maximum, numericality: { greater_than: 0, less_than: 1000 } - default_scope { order(id: :asc) } + default_scope { order(order: :asc, id: :asc) } private diff --git a/app/policies/evaluation_exercise_policy.rb b/app/policies/evaluation_exercise_policy.rb index 43d72dcac2..5bdfb25a1f 100644 --- a/app/policies/evaluation_exercise_policy.rb +++ b/app/policies/evaluation_exercise_policy.rb @@ -10,6 +10,10 @@ def permitted_attributes %i[visible_score] end + def update? + course_admin? + end + private def course_admin? diff --git a/app/policies/evaluation_policy.rb b/app/policies/evaluation_policy.rb index 4f0eb8c80d..1b621d78c5 100644 --- a/app/policies/evaluation_policy.rb +++ b/app/policies/evaluation_policy.rb @@ -15,10 +15,6 @@ def add_users? course_admin? end - def score_items? - course_admin? - end - def manage_scores? course_admin? end diff --git a/app/policies/score_item_policy.rb b/app/policies/score_item_policy.rb index d5cde97084..eec77dd944 100644 --- a/app/policies/score_item_policy.rb +++ b/app/policies/score_item_policy.rb @@ -1,4 +1,3 @@ -# This policy contains no query methods, as they are covered by score_items? on the evaluation policy. class ScoreItemPolicy < ApplicationPolicy class Scope < Scope def resolve @@ -17,12 +16,4 @@ def resolve end end end - - def permitted_attributes_for_create - %i[evaluation_exercise_id maximum name visible description] - end - - def permitted_attributes_for_update - %i[maximum name visible description] - end end diff --git a/app/views/evaluations/_score_items.html.erb b/app/views/evaluations/_score_items.html.erb index 0aef24abc0..8e916c279c 100644 --- a/app/views/evaluations/_score_items.html.erb +++ b/app/views/evaluations/_score_items.html.erb @@ -25,12 +25,6 @@ <%= t("score_items.new.hide_all") %> <% end %> -
  • - - - <%= t 'score_items.new.add_all' %> - -
    • @@ -49,12 +43,3 @@ <% end %> - diff --git a/app/views/score_items/_copy.html.erb b/app/views/score_items/_copy.html.erb deleted file mode 100644 index 9a80affc82..0000000000 --- a/app/views/score_items/_copy.html.erb +++ /dev/null @@ -1,26 +0,0 @@ -
    -
    -

    <%= t("score_items.exercise.copy_title", name: evaluation_exercise.exercise.name) %>

    - -
    - - <%= form_for(:copy, url: copy_evaluation_score_items_path(@evaluation), html: { class: 'form-horizontal' }, namespace: evaluation_exercise.id, remote: true) do |f| %> -
    - <%= f.hidden_field :to, value: evaluation_exercise.id %> -
    - <%= f.label t("score_items.copy.exercise"), :class => "col-sm-3 col-form-label" %> -
    - <%= f.collection_select :from, - evaluation.evaluation_exercises.filter { |o| o.id != evaluation_exercise.id }, - :id, - ->(e) { e.exercise.name }, - { include_blank: t("score_items.copy.choose") }, - { class: "form-select", required: true } %> -
    -
    -
    -
    - <%= f.submit t("score_items.copy.copy"), class: "btn btn-filled" %> -
    - <% end %> -
    diff --git a/app/views/score_items/_exercise.html.erb b/app/views/score_items/_exercise.html.erb index 4d88cf0b35..5cbc1ff930 100644 --- a/app/views/score_items/_exercise.html.erb +++ b/app/views/score_items/_exercise.html.erb @@ -3,22 +3,27 @@ <% end %> <% maximum_score = evaluation_exercise.maximum_score %>
    -

    <%= evaluation_exercise.exercise.name %>

    +

    + <%= evaluation_exercise.exercise.name %> +

    - +
    "> - <% if evaluation_exercise.score_items.empty? %> - + <% else %> <% evaluation_exercise.score_items.each do |score_item| %> @@ -27,84 +32,46 @@ - <% end %> <% end %> - + + -
    <%= ScoreItem.human_attribute_name(:name) %> <%= ScoreItem.human_attribute_name(:description) %> <%= ScoreItem.human_attribute_name(:maximum) %> <%= ScoreItem.human_attribute_name(:visible) %>
    <%= t 'score_items.exercise.nothing' %> + <%= t 'score_items.exercise.nothing' %> +
    <%= markdown score_item.description %> <%= format_score score_item.maximum %> - <%= form_for [@evaluation, score_item], namespace: "#{evaluation_exercise.id}_#{score_item.id}", method: :patch, remote: :true do |f| %> - <%= f.check_box :visible, class: "form-check-input visibility-checkbox", title: t(".visibility") %> - <% end %> - - <%= link_to "#edit-form-#{score_item.id}", title: t('.edit_title'), class: 'btn btn-icon edit-button', - data: { "bs-toggle": "modal" } do %> - - <% end %> - <%= button_to evaluation_score_item_path(@evaluation, score_item), - title: t('.delete_title'), - method: :delete, - remote: true, - data: { confirm: t('general.are_you_sure') }, - class: 'btn btn-icon btn-icon-filled d-btn-danger' do %> - + <% if score_item.visible %> + + <% else %> + <% end %>
    <%= t '.max' %><%= t '.max' %> + + <%= t ".add" %> + + <%= format_score maximum_score %> - <%= form_for evaluation_exercise, namespace: evaluation_exercise.id, method: :patch, remote: :true do |f| %> - <%= f.check_box :visible_score, - class: "form-check-input total-visibility-checkbox", - disabled: maximum_score.nil?, - title: t(".total_visibility") %> - <% end %> - - <%= link_to "#copy-form-#{evaluation_exercise.id}", - class: 'btn btn-icon', - title: t('.copy_title', name: evaluation_exercise.exercise.name), - data: { "bs-toggle": 'modal' } do %> - - <% end %> - <%= link_to "#new-form-#{evaluation_exercise.id}", - class: 'btn btn-icon btn-icon-filled bg-primary', - title: t('.add_title', name: evaluation_exercise.exercise.name), - data: { "bs-toggle": 'modal' } do %> - + <% if evaluation_exercise.visible_score %> + + <% else %> + <% end %>
    - <% evaluation_exercise.score_items.each do |score_item| %> - - <% end %> - - + + total-visible="true" + <% end %> + >
    diff --git a/app/views/score_items/_form.html.erb b/app/views/score_items/_form.html.erb deleted file mode 100644 index 17f39b766c..0000000000 --- a/app/views/score_items/_form.html.erb +++ /dev/null @@ -1,60 +0,0 @@ -
    -
    -

    <%= title %>

    - -
    - <% options = {html: { class: 'form-horizontal' }}.merge(form_options) %> - <% score_item = local_assigns.fetch(:score_item, local_assigns[:models]&.last) %> - <%= form_for(local_assigns.fetch(:models, ScoreItem.new), options) do |f| %> -
    - <% if score_item&.errors&.any? %> -
    -

    <%= t('errors.validation_errors', :count => score_item.errors.count) %>

    -
      - <% score_item.errors.full_messages.each do |message| %> -
    • <%= message %>
      • - <% end %> -
    -
    - <% end %> - <% if score_item&.scores.present? %> -
    - <%= t '.modify-help' %> -
    - <% end %> - <% if score_item&.new_record? && local_assigns[:models] %> - <%= f.hidden_field :evaluation_exercise_id, value: evaluation_exercise.id %> - <% end %> -
    - <%= f.label :name, :class => "col-sm-4 col-form-label" %> -
    <%= f.text_field :name, required: true, class: "form-control" %>
    -
    -
    - <%= f.label :description, :class => "col-sm-4 col-form-label" %> -
    <%= f.text_area :description, class: "form-control", rows: "2" %>
    - <%= t "score_items.form.description-help_html" %> -
    -
    - <%= f.label :maximum, :class => "col-sm-4 col-form-label" %> -
    <%= f.number_field :maximum, step: 0.25, min: 0.25, max: 999.75, required: true, class: "form-control" %>
    - <%= t "score_items.form.score-help" %> -
    -
    - <%= f.label :visible, class: 'col-sm-4 col-form-label' %> -
    -
    - <%= f.check_box :visible, class: "form-check-input" %> - <%= f.label :visible, t("score_items.form.visible-help"), class: "form-check-label" %> -
    -
    -
    -
    -
    - <% if score_item&.new_record? %> - <%= f.submit t("score_items.form.add"), class: "btn btn-filled" %> - <% else %> - <%= f.submit t("score_items.form.save"), class: "btn btn-filled" %> - <% end %> -
    - <% end %> -
    diff --git a/app/views/score_items/_score_item.json.jbuilder b/app/views/score_items/_score_item.json.jbuilder deleted file mode 100644 index a19163795c..0000000000 --- a/app/views/score_items/_score_item.json.jbuilder +++ /dev/null @@ -1 +0,0 @@ -json.extract! score_item, :id, :evaluation_exercise_id, :maximum, :name, :visible, :description diff --git a/app/views/score_items/add_all.js.erb b/app/views/score_items/add_all.js.erb deleted file mode 100644 index 6c41644f4e..0000000000 --- a/app/views/score_items/add_all.js.erb +++ /dev/null @@ -1,2 +0,0 @@ -bootstrap.Modal.getOrCreateInstance(document.querySelector("#add-score-item-to-all")).hide(); -dodona.setHTMLExecuteScripts(document.querySelector("#items-step"), "<%= raw escape_javascript(render partial: 'evaluations/score_items') %>"); diff --git a/app/views/score_items/index.js.erb b/app/views/score_items/index.js.erb index 26c8b6b0ae..6654a45648 100644 --- a/app/views/score_items/index.js.erb +++ b/app/views/score_items/index.js.erb @@ -1,11 +1,2 @@ -<%# Refreshes the score items for one exercise. %> -vissibleModal = document.querySelector(".modal-<%= evaluation_exercise.id %>.show") -if (vissibleModal){ - vissibleModal.addEventListener("hidden.bs.modal", () => { - dodona.setHTMLExecuteScripts(document.querySelector("#card-<%= evaluation_exercise.id %>"), "<%= escape_javascript(render 'score_items/exercise', evaluation_exercise: evaluation_exercise, new: new) %>"); - }, { once: true }); - bootstrap.Modal.getOrCreateInstance(vissibleModal).hide(); -} else { - dodona.setHTMLExecuteScripts(document.querySelector("#card-<%= evaluation_exercise.id %>"), "<%= escape_javascript(render 'score_items/exercise', evaluation_exercise: evaluation_exercise, new: new) %>"); -} +dodona.setHTMLExecuteScripts(document.querySelector("#card-<%= evaluation_exercise.id %>"), "<%= escape_javascript(render 'score_items/exercise', evaluation_exercise: evaluation_exercise, new: new) %>"); dodona.setHTMLExecuteScripts(document.querySelector(".summary-text"), "<%= t('score_items.new.summary_html', count: @evaluation.score_items.count, score: format_score(@evaluation.maximum_score)) %>"); diff --git a/app/views/score_items/show.json.jbuilder b/app/views/score_items/show.json.jbuilder deleted file mode 100644 index 870fcd8d3d..0000000000 --- a/app/views/score_items/show.json.jbuilder +++ /dev/null @@ -1 +0,0 @@ -json.partial! @score_item diff --git a/config/locales/js/en.yml b/config/locales/js/en.yml index 8a5c683113..93b7a023fb 100644 --- a/config/locales/js/en.yml +++ b/config/locales/js/en.yml @@ -294,3 +294,24 @@ en: dolos: view_report: Open plagiarism report unsupported_language: "%{lang} is not supported by the plagiarism checker." + score_items: + name: Name + description: Description + visible: Visible + maximum: Maximum + description_help: A description is optional. Markdown formatting can be used. This is visible to the students. + visible_help: Make the score item visible to students once the evaluation is released. + maximum_help: The maximum grade for this score item. The grade should be between 0 and 1000, and works in increments of 0.25. + modified_warning: You have changed the maximum score of one or more score items. This will mark all completed evaluations with this score item as uncompleted. + deleted_warning: You have deleted one or more score items. This will also delete all scores for these items. + validation_warning: All score items must have a name and a maximum score, and the maximum score must be between 0 and 1000. + save: Save + cancel: Cancel + total_visible: Make the total score visible to students once the evaluation is released. + jspreadsheet: + copy: Copy... + deleteRow: Delete row + deleteSelectedRows: Delete selected rows + insertNewRowAfter: Insert new row after + insertNewRowBefore: Insert new row before + paste: Paste... diff --git a/config/locales/js/nl.yml b/config/locales/js/nl.yml index 33ea0931df..f55652803e 100644 --- a/config/locales/js/nl.yml +++ b/config/locales/js/nl.yml @@ -294,4 +294,25 @@ nl: dolos: view_report: Rapport bekijken unsupported_language: "%{lang} wordt niet ondersteund door Dolos" + score_items: + name: Naam + description: Beschrijving + maximum: Maximum + visible: Zichtbaar + description_help: Een beschrijving is optioneel en kan in Markdown geschreven worden. Dit is zichtbaar voor de studenten. + visible_help: Maak het scoreonderdeel zichtbaar voor studenten eens de evaluatie vrijgegeven is. + maximum_help: De maximumscore voor dit scoreonderdeel. Dit moet een getal zijn tussen 0 en 1000 en gaat in stappen van 0.25. + save: Opslaan + cancel: Annuleren + modified_warning: "Je hebt de maximumscore van een of meerdere scoreonderdelen aangepast. Alle afgewerkte evaluaties met dit scoreonderdeel zullen terug als onafgewerkt gemarkeerd worden." + deleted_warning: "Je hebt een of meerdere scoreonderdelen verwijderd. Dit zal ook de bijhorende scores van de studenten verwijderen." + validation_warning: "Alle scoreonderdelen moeten een naam en een maximumscore hebben. De maximumscore moet een getal zijn tussen 0 en 1000." + total_visible: Maak totaal score zichtbaar voor studenten eens de evaluatie vrijgegeven is. + jspreadsheet: + copy: Kopieer... + deleteRow: Verwijder rij + deleteSelectedRows: Verwijder geselecteerde rijen + insertNewRowAfter: Voeg nieuwe rij toe na deze + insertNewRowBefore: Voeg nieuwe rij toe voor deze + paste: Plak... diff --git a/config/locales/views/score_items/en.yml b/config/locales/views/score_items/en.yml index 11b39e0d54..646cdacade 100644 --- a/config/locales/views/score_items/en.yml +++ b/config/locales/views/score_items/en.yml @@ -1,17 +1,5 @@ en: score_items: - form: - description-help_html: A description is optional. Markdown formatting can be used. This is visible to the students. - visible-help: Make the score item visible to students once the evaluation is released. - save: Save - add: Add - close: Close - modify-help: If you change the maximum, all completed evaluaties with this score item will be marked as uncompleted. - score-help: The maximum grade for this score item. The grade should be greater than 0, and works in increments of 0.25. - copy: - copy: Copy - exercise: Exercise - choose: "-- Choose an exercise --" table: total: "Total" total-description: "Sum of all score items" @@ -44,14 +32,9 @@ en: info: Dodona allows you to grade the submissions in this evaluation. This is optional and can always be changed later. exercise: nothing: There are no score items for this exercise yet. You can only grade exercises with at least one score item. - copy_title: Copy score item from another exercise to %{name} - add_title: Create new score item for %{name} - all: Add score item to all exercises - max: Maximum score - edit_title: Edit this score item - delete_title: Delete this score item - edit_item_title: "Edit %{title}" + max: Total score total_visibility: > Show or hide the calculated total score in Dodona for this exercise when the feedback is released. Individual score items are still visible if they are marked as such. - visibility: Show or hide this score item. + edit: Edit + add: Add score items diff --git a/config/locales/views/score_items/nl.yml b/config/locales/views/score_items/nl.yml index cc57f3916e..a088c64121 100644 --- a/config/locales/views/score_items/nl.yml +++ b/config/locales/views/score_items/nl.yml @@ -1,17 +1,5 @@ nl: score_items: - form: - description-help_html: Een beschrijving is optioneel en kan in Markdown geschreven worden. Dit is zichtbaar voor de studenten. - visible-help: Maak het scoreonderdeel zichtbaar voor studenten eens de evaluatie vrijgegeven is. - save: Opslaan - add: Toevoegen - close: Sluiten - modify-help: Als je het maximum aanpast zullen alle afgewerkte evaluaties met dit scoreonderdeel terug als onafgewerkt gemarkeerd worden. - score-help: Het maximaal aantal punten voor dit scoreonderdeel. Het maximum moet groter dan 0 zijn, en werkt in stappen van 0,25. - copy: - copy: Kopiëren - exercise: Oefening - choose: "-- Kies een oefening --" table: total: "Totaal" total-description: "Som van alle scoreonderdelen" @@ -44,14 +32,9 @@ nl: info: Je kan op Dodona punten geven aan de oplossingen in deze evaluatie. Dit is optioneel en kan later altijd aangepast worden. exercise: nothing: Er zijn nog geen scoreonderdelen voor deze oefening. Je kan pas punten geven als er minstens één scoreonderdeel is. - copy_title: Scoreonderdeel kopiëren van een andere oefening naar %{name} - add_title: Nieuw scoreonderdeel toevoegen aan %{name} - all: Scoreonderdeel toevoegen aan alle oefeningen - max: Maximumscore - edit_title: Bewerk dit scoreonderdeel - delete_title: Verwijder dit scoreonderdeel - edit_item_title: "%{title} bewerken" + max: Totaal total_visibility: > Toon of verberg de berekende totaalscore in Dodona voor deze oefening als de feedback vrijgegeven wordt. Individuele scoreonderdelen zijn nog wel zichtbaar als ze als zodanig gemarkeerd zijn. - visibility: Toon of verberg dit scoreonderdeel. + edit: Bewerken + add: Voeg scoreonderdelen toe diff --git a/config/routes.rb b/config/routes.rb index 13a9d5269b..4646495575 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -223,10 +223,6 @@ post 'modify_grading_visibility' end resources :feedbacks, only: %i[show edit update] - resources :score_items, only: %i[create destroy update] do - post 'copy', on: :collection - post 'add_all', on: :collection - end resources :scores, only: %i[show create update destroy] end resources :feedbacks, only: %i[show edit update] do diff --git a/db/migrate/20240807134422_add_order_to_score_items.rb b/db/migrate/20240807134422_add_order_to_score_items.rb new file mode 100644 index 0000000000..3d9b192dc7 --- /dev/null +++ b/db/migrate/20240807134422_add_order_to_score_items.rb @@ -0,0 +1,5 @@ +class AddOrderToScoreItems < ActiveRecord::Migration[7.1] + def change + add_column :score_items, :order, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index 270ddcaeae..091f632c94 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -439,6 +439,7 @@ t.text "description" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.integer "order" t.index ["evaluation_exercise_id"], name: "index_score_items_on_evaluation_exercise_id" end diff --git a/package.json b/package.json index f468aaf512..5be928c0a8 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,7 @@ "flatpickr": "^4.6.13", "fscreen": "^1.2.0", "glightbox": "^3.2.0", + "jspreadsheet-ce": "^4.2.1", "lit": "^3.2.1", "node-polyglot": "^2.6.0", "sass": "^1.83.0", diff --git a/test/controllers/evaluation_exercise_controller_test.rb b/test/controllers/evaluation_exercise_controller_test.rb index fed31f2751..81e045778a 100644 --- a/test/controllers/evaluation_exercise_controller_test.rb +++ b/test/controllers/evaluation_exercise_controller_test.rb @@ -5,12 +5,11 @@ def setup @evaluation = create :evaluation, :with_submissions @staff_member = users(:staff) @evaluation.series.course.administrating_members << @staff_member + @exercise = @evaluation.evaluation_exercises.first sign_in @staff_member end test 'can update visibility as course admin' do - evaluation_exercise = @evaluation.evaluation_exercises.first - [ [@staff_member, :success], [users(:student), :forbidden], @@ -20,25 +19,141 @@ def setup ].each do |user, expected| sign_in user if user.present? - evaluation_exercise.update!(visible_score: true) + @exercise.update!(visible_score: true) - assert_predicate evaluation_exercise, :visible_score? + assert_predicate @exercise, :visible_score? - patch evaluation_exercise_path(evaluation_exercise, format: :js), params: { + patch evaluation_exercise_path(@exercise, format: :js), params: { evaluation_exercise: { visible_score: false } } assert_response expected - evaluation_exercise.reload + @exercise.reload + if expected == :success + assert_not @exercise.visible_score? + else + assert_predicate @exercise, :visible_score? + end + + sign_out user if user.present? + end + end + + test 'should update all score items if course administrator' do + score_items = create_list :score_item, 3, evaluation_exercise: @exercise, + description: 'Before test', + maximum: '10.0' + + [ + [@staff_member, :success], + [users(:student), :forbidden], + [create(:staff), :forbidden], + [users(:zeus), :success], + [nil, :unauthorized] + ].each do |user, expected| + sign_in user if user.present? + patch evaluation_exercise_path(@exercise, format: :json), params: { + evaluation_exercise: { + score_items: [ + { id: score_items[0].id, name: 'edited', description: 'After test', maximum: '20.0' }, + { name: 'new', description: 'new value', maximum: '25.0' } + ] + } + } + + assert_response expected + + @exercise.score_items.reload if expected == :success - assert_not evaluation_exercise.visible_score? + assert_equal 2, @exercise.score_items.count + assert_equal 'After test', @exercise.score_items.first.description + assert_in_delta(20.0, @exercise.score_items.first.maximum) + assert_equal 'new', @exercise.score_items.last.name + assert_equal 'new value', @exercise.score_items.last.description + assert_in_delta(25.0, @exercise.score_items.last.maximum) + + # reset + @exercise.score_items.each(&:destroy) + score_items = create_list :score_item, 3, evaluation_exercise: @exercise, + description: 'Before test', + maximum: '10.0' else - assert_predicate evaluation_exercise, :visible_score? + assert_equal 3, @exercise.score_items.count end sign_out user if user.present? end end + + test 'should not create score item for invalid data' do + # Missing data + patch evaluation_exercise_path(@exercise, format: :json), params: { + evaluation_exercise: { + score_items: [ + { name: 'new' } + ] + } + } + + assert_response :unprocessable_entity + + # Negative maximum + patch evaluation_exercise_path(@exercise, format: :json), params: { + evaluation_exercise: { + score_items: [ + { name: 'new', description: 'new value', maximum: '-20.0' } + ] + } + } + + assert_response :unprocessable_entity + end + + test 'should not update score item for invalid data' do + score_item = create :score_item, evaluation_exercise: @exercise + + # Negative maximum + patch evaluation_exercise_path(@exercise, format: :json), params: { + evaluation_exercise: { + score_items: [ + { id: score_item.id, maximum: '-20.0' } + ] + } + } + + assert_response :unprocessable_entity + end + + test 'should delete score item if course administrator' do + assert_equal 0, @exercise.score_items.count + + [ + [@staff_member, :success], + [users(:student), :forbidden], + [create(:staff), :forbidden], + [users(:zeus), :success], + [nil, :unauthorized] + ].each do |user, expected| + create :score_item, evaluation_exercise: @exercise, + description: 'Code re-use', + maximum: '10.0' + + assert_equal 1, @exercise.score_items.count + sign_in user if user.present? + patch evaluation_exercise_path(@exercise, format: :json), params: { + evaluation_exercise: { + score_items: [] + } + }, as: :json + + assert_response expected + @exercise.score_items.reload + assert_equal 0, @exercise.score_items.count if response == :success + + sign_out user if user.present? + @exercise.update!(score_items: []) + end + end end diff --git a/test/controllers/score_items_controller_test.rb b/test/controllers/score_items_controller_test.rb deleted file mode 100644 index 781fcc97e7..0000000000 --- a/test/controllers/score_items_controller_test.rb +++ /dev/null @@ -1,180 +0,0 @@ -require 'test_helper' - -class ScoreItemsControllerTest < ActionDispatch::IntegrationTest - def setup - @evaluation = create :evaluation, :with_submissions - @staff_member = users(:staff) - @evaluation.series.course.administrating_members << @staff_member - sign_in @staff_member - end - - test 'should copy score items if course administrator' do - from = @evaluation.evaluation_exercises.first - create :score_item, evaluation_exercise: from - create :score_item, evaluation_exercise: from - - [ - [@staff_member, :success], - [users(:student), :forbidden], - [create(:staff), :forbidden], - [users(:zeus), :success], - [nil, :unauthorized] - ].each do |user, expected| - to = create :evaluation_exercise, evaluation: @evaluation - sign_in user if user.present? - post copy_evaluation_score_items_path(@evaluation, format: :js), params: { - copy: { - from: from.id, - to: to.id - } - } - - assert_response expected - assert_equal 2, to.score_items.count if expected == :success - - sign_out user if user.present? - end - end - - test 'should add score items to all if course administrator' do - [ - [@staff_member, :ok], - [users(:student), :no], - [create(:staff), :no], - [users(:zeus), :ok], - [nil, :no] - ].each do |user, expected| - sign_in user if user.present? - post add_all_evaluation_score_items_path(@evaluation, format: :js), params: { - score_item: { - name: 'Test', - description: 'Test', - maximum: '20.0' - } - } - @evaluation.evaluation_exercises.reload - @evaluation.evaluation_exercises.each do |e| - if expected == :ok - assert_equal 1, e.score_items.length - e.update!(score_items: []) - end - - assert_empty e.score_items - end - sign_out user if user.present? - end - end - - test 'should update score item if course administrator' do - exercise = @evaluation.evaluation_exercises.first - score_item = create :score_item, evaluation_exercise: exercise, - description: 'Before test', - maximum: '10.0' - - [ - [@staff_member, :success], - [users(:student), :forbidden], - [create(:staff), :forbidden], - [users(:zeus), :success], - [nil, :unauthorized] - ].each do |user, expected| - sign_in user if user.present? - patch evaluation_score_item_path(@evaluation, score_item, format: :json), params: { - score_item: { - description: 'After test', - maximum: '20.0' - } - } - - assert_response expected - sign_out user if user.present? - end - end - - test 'should create score item if course administrator' do - [ - [@staff_member, :created], - [users(:student), :forbidden], - [create(:staff), :forbidden], - [users(:zeus), :created], - [nil, :unauthorized] - ].each do |user, expected| - sign_in user if user.present? - post evaluation_score_items_path(@evaluation, format: :json), params: { - score_item: { - name: 'Code re-use', - description: 'After test', - maximum: '10.0', - evaluation_exercise_id: @evaluation.evaluation_exercises.first.id - } - } - - assert_response expected - sign_out user if user.present? - end - end - - test 'should not create score item for invalid data' do - # Missing data - post evaluation_score_items_path(@evaluation, format: :json), params: { - score_item: { - name: 'Code re-use', - evaluation_exercise_id: @evaluation.evaluation_exercises.first.id - } - } - - assert_response :unprocessable_entity - - # Negative maximum - post evaluation_score_items_path(@evaluation, format: :json), params: { - score_item: { - name: 'Code re-use', - maximum: '-20.0', - evaluation_exercise_id: @evaluation.evaluation_exercises.first.id - } - } - - assert_response :unprocessable_entity - end - - test 'should not update score item for invalid data' do - score_item = create :score_item, evaluation_exercise: @evaluation.evaluation_exercises.sample - # Negative maximum - patch evaluation_score_item_path(@evaluation, score_item, format: :json), params: { - score_item: { - maximum: '-20.0' - } - } - - assert_response :unprocessable_entity - end - - test 'should delete score item if course administrator' do - exercise = @evaluation.evaluation_exercises.first - - assert_equal 0, exercise.score_items.count - - [ - [@staff_member, :success], - [users(:student), :forbidden], - [create(:staff), :forbidden], - [users(:zeus), :success], - [nil, :unauthorized] - ].each do |user, expected| - score_item = create :score_item, evaluation_exercise: exercise, - description: 'Code re-use', - maximum: '10.0' - - assert_equal 1, exercise.score_items.count - sign_in user if user.present? - delete evaluation_score_item_path(@evaluation, score_item, format: :json) - - assert_response expected - exercise.score_items.reload - assert_equal 0, exercise.score_items.count if response == :success - - sign_out user if user.present? - exercise.update!(score_items: []) - end - end -end diff --git a/test/factories/score_items.rb b/test/factories/score_items.rb index aa51c4cf66..1597a21278 100644 --- a/test/factories/score_items.rb +++ b/test/factories/score_items.rb @@ -10,6 +10,7 @@ # description :text(65535) # created_at :datetime not null # updated_at :datetime not null +# order :integer # FactoryBot.define do factory :score_item do diff --git a/test/javascript/utilities.test.js b/test/javascript/utilities.test.js index 33cba86807..9c16a5aa39 100644 --- a/test/javascript/utilities.test.js +++ b/test/javascript/utilities.test.js @@ -2,6 +2,7 @@ import { updateArrayURLParameter, updateURLParameter, getURLParameter, getArrayURLParameter, delay, createDelayer } from "utilities.ts"; +import { convertToFloatRepresentation } from "../../app/assets/javascripts/utilities"; jest.useFakeTimers(); diff --git a/test/models/score_item_test.rb b/test/models/score_item_test.rb index 09fc04490d..f51cbc2f13 100644 --- a/test/models/score_item_test.rb +++ b/test/models/score_item_test.rb @@ -10,6 +10,7 @@ # description :text(65535) # created_at :datetime not null # updated_at :datetime not null +# order :integer # require 'test_helper' diff --git a/test/system/score_items_test.rb b/test/system/score_items_test.rb deleted file mode 100644 index b8a1866f02..0000000000 --- a/test/system/score_items_test.rb +++ /dev/null @@ -1,39 +0,0 @@ -require 'capybara/minitest' -require 'application_system_test_case' - -class ScoreItemsTest < ApplicationSystemTestCase - include Devise::Test::IntegrationHelpers - # Make the Capybara DSL available in all integration tests - include Capybara::DSL - # Make `assert_*` methods behave like Minitest assertions - include Capybara::Minitest::Assertions - - def setup - @evaluation = create :evaluation, :with_submissions - @staff_member = create :staff - @evaluation.series.course.administrating_members << @staff_member - sign_in @staff_member - @exercise = @evaluation.evaluation_exercises.first - @score_item = create :score_item, evaluation_exercise: @exercise, - description: 'Before test', - maximum: '400.0', - visible: false - end - - test 'updating score item works' do - visit(edit_evaluation_path(id: @evaluation.id)) - - # Ensure we don't accidentally test nothing - assert_no_text '314.25' - - # Click the edit button of the score item - find("a[href=\"#edit-form-#{@score_item.id}\"]").click - # Change value of score item - find(id: "#{@exercise.id}_score_item_maximum").fill_in with: '314.25' - # Save our changes to the score item - find(id: "#{@exercise.id}_edit_score_item_#{@score_item.id}").find('input[type=submit]').click - - # Check that the score has been updated on the page - assert_text '314.25' - end -end diff --git a/yarn.lock b/yarn.lock index ee6c8ca56a..01e9243676 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1736,6 +1736,11 @@ resolved "https://registry.yarnpkg.com/@jsplumb/browser-ui/-/browser-ui-6.2.10.tgz#86b85ed42110563d2816e3712677cdbbbf33366f" integrity sha512-trk++mK5q6hceJL79teemzcilJ+8DrZT/lMK0+B80AtHqZHr0YwMCf+so2JBb2Z/MDZ0fUEU9MbELY6OPhhs5g== +"@jspreadsheet/formula@^2.0.2": + version "2.0.2" + resolved "https://registry.yarnpkg.com/@jspreadsheet/formula/-/formula-2.0.2.tgz#e0529f275c086fd7d34179b50450a98aeb56fd58" + integrity sha512-PDQYf9REQA53I7tVYkvkeyQxrd5jcjUeHgItYnRpjN2QiIQwawSqBDtGGEVQTSboTG+JwgGCuhvOpj7FxeKwew== + "@lezer/common@^1.0.0", "@lezer/common@^1.0.2", "@lezer/common@^1.1.0", "@lezer/common@^1.2.0", "@lezer/common@^1.2.3": version "1.2.3" resolved "https://registry.yarnpkg.com/@lezer/common/-/common-1.2.3.tgz#138fcddab157d83da557554851017c6c1e5667fd" @@ -6054,6 +6059,19 @@ json5@2.x, json5@^2.1.2, json5@^2.2.3: resolved "https://registry.yarnpkg.com/json5/-/json5-2.2.3.tgz#78cd6f1a19bdc12b73db5ad0c61efd66c1e29283" integrity sha512-XmOWe7eyHYH14cLdVPoyg+GOH3rYX++KpzrylJwSW98t3Nk+U8XOl8FWKOgwtzdb8lXGf6zYwDUzeHMWfxasyg== +jspreadsheet-ce@^4.2.1: + version "4.2.1" + resolved "https://registry.yarnpkg.com/jspreadsheet-ce/-/jspreadsheet-ce-4.2.1.tgz#66cba61fb28d65d6cf89e6aa495aecd89f63a710" + integrity sha512-kQ+bUwH0MEeyr3Ojdd+tB0BJik3NsnQlhC1E52uwrURvqBu3GnrDtRZZS3cAwtL0FzHBZDsJJB9afTdReBm6Hg== + dependencies: + "@jspreadsheet/formula" "^2.0.2" + jsuites "^5.3.0" + +jsuites@^5.3.0: + version "5.4.6" + resolved "https://registry.yarnpkg.com/jsuites/-/jsuites-5.4.6.tgz#0d8dc2aa3cdffda4ce0f07df8aa3a2177b3a3cec" + integrity sha512-/Do37lqZ+EhBKvWi3L1r7wHjP9PHAtjDPLNepp84Pqi4pWrH6ZisTuJZyI6SRHYqshsfMr+cg5CkPbPC6J6o4Q== + keyv@^4.5.3, keyv@^4.5.4: version "4.5.4" resolved "https://registry.yarnpkg.com/keyv/-/keyv-4.5.4.tgz#a879a99e29452f942439f2a405e3af8b31d4de93"