diff --git a/app/assets/javascripts/components/input_table.ts b/app/assets/javascripts/components/input_table.ts new file mode 100644 index 0000000000..eb3286b790 --- /dev/null +++ b/app/assets/javascripts/components/input_table.ts @@ -0,0 +1,304 @@ +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"; + +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 DodonaElement { + @property({ type: String }) + route: string = ""; + @property({ type: Array, attribute: "score-items" }) + scoreItems: ScoreItem[] = []; + + tableRef: Ref = createRef(); + table: JspreadsheetInstance; + + @property({ state: true }) + hasErrors: boolean = false; + + 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({ + 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.hasErrors = false; + } + this.dispatchEvent(new Event("cancel")); + } + + + render(): TemplateResult { + return html` + ${this.hasErrors ? html`
${i18n.t("js.score_items.validation_warning")}
` : ""} +
+
+ + +
+ `; + } +} diff --git a/app/assets/javascripts/i18n/translations.json b/app/assets/javascripts/i18n/translations.json index fa881ec177..928841639d 100644 --- a/app/assets/javascripts/i18n/translations.json +++ b/app/assets/javascripts/i18n/translations.json @@ -330,6 +330,28 @@ "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", + "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 +847,28 @@ "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", + "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..b05d4f7b92 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,31 @@ function initTotalVisibilityCheckboxes(element: HTMLElement): void { }); } -function initItemVisibilityCheckboxes(element: HTMLElement): void { - commonCheckboxInit(element, ".visibility-checkbox", checked => { - return { - - score_item: { - visible: checked - } - }; - }); -} - export function initVisibilityCheckboxes(element: HTMLElement): void { initTotalVisibilityCheckboxes(element); - initItemVisibilityCheckboxes(element); +} + +export function initEditButton(element: HTMLElement): void { + const editBtn = element.querySelector(".edit-btn") as HTMLAnchorElement; + const table = element.querySelector(".score-items-table") as HTMLTableElement; + const allRowsButLast = table.querySelectorAll("tr:not(:last-child)"); + const tableHeader = table.querySelector("thead") as HTMLTableSectionElement; + const form = element.querySelector("d-score-item-input-table") as ScoreItemInputTable; + + const activateEditMode = (e: MouseEvent): void => { + e.preventDefault(); + table.classList.add("d-none"); + editBtn.classList.add("d-none"); + form.classList.remove("d-none"); + }; + + editBtn.addEventListener("click", activateEditMode); + allRowsButLast.forEach(row => row.addEventListener("click", activateEditMode)); + tableHeader.addEventListener("click", activateEditMode); + + form.addEventListener("cancel", () => { + table.classList.remove("d-none"); + editBtn.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/models/score_items.css.scss b/app/assets/stylesheets/models/score_items.css.scss index b4638df288..178dd6f3f5 100644 --- a/app/assets/stylesheets/models/score_items.css.scss +++ b/app/assets/stylesheets/models/score_items.css.scss @@ -5,6 +5,8 @@ } .score-items-table { + cursor: text; + td.description { max-width: 150px; @@ -88,3 +90,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/score_items_controller.rb b/app/controllers/score_items_controller.rb index 0b67381489..d5ad2e74a6 100644 --- a/app/controllers/score_items_controller.rb +++ b/app/controllers/score_items_controller.rb @@ -4,24 +4,6 @@ class ScoreItemsController < ApplicationController 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 @@ -50,17 +32,31 @@ def update 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 + def update_all + @evaluation_exercise = EvaluationExercise.find(params[:evaluation_exercise_id]) + Rails.logger.debug { "update_all: #{params[:score_items]}" } + authorize @evaluation_exercise, :update? + + ScoreItem.transaction do + new_items = params[:score_items].filter { |item| item[:id].blank? } + updated_items = params[: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 + end + + respond_to do |format| + @evaluation.reload + format.js { render 'score_items/index', locals: { new: nil, evaluation_exercise: @evaluation_exercise.reload } } + format.json { head :no_content } end - @evaluation.reload end def destroy 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/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 @@ - diff --git a/app/views/score_items/_exercise.html.erb b/app/views/score_items/_exercise.html.erb index 4d88cf0b35..26cf492824 100644 --- a/app/views/score_items/_exercise.html.erb +++ b/app/views/score_items/_exercise.html.erb @@ -3,7 +3,12 @@ <% end %> <% maximum_score = evaluation_exercise.maximum_score %>
    -

    <%= evaluation_exercise.exercise.name %>

    +

    + <%= evaluation_exercise.exercise.name %> + + <%= t ".edit" %> + +

    @@ -12,13 +17,19 @@ - <% if evaluation_exercise.score_items.empty? %> - + <% else %> <% evaluation_exercise.score_items.each do |score_item| %> @@ -27,22 +38,10 @@ - @@ -55,56 +54,26 @@ -
    <%= 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 %>
    <%= 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 %> - +
    + <%= f.check_box :visible_score, + class: "form-check-input total-visibility-checkbox", + disabled: maximum_score.nil?, + title: t(".total_visibility"), + 'data-bs-toggle': "tooltip" %> +
    <% end %>
    - <% evaluation_exercise.score_items.each do |score_item| %> - - <% 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 @@ - 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/config/locales/js/en.yml b/config/locales/js/en.yml index 8a5c683113..05b274c470 100644 --- a/config/locales/js/en.yml +++ b/config/locales/js/en.yml @@ -294,3 +294,23 @@ 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 + 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..b587e038d0 100644 --- a/config/locales/js/nl.yml +++ b/config/locales/js/nl.yml @@ -294,4 +294,24 @@ 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." + 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..42b7e750ba 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}" 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..e3bf8ca627 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" 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..b99dff259f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -223,11 +223,14 @@ 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 :score_items, only: %i[create destroy update] resources :scores, only: %i[show create update destroy] + + resources :evaluation_exercise do + resources :score_items do + patch '/', on: :collection, to: 'score_items#update_all' + end + end end resources :feedbacks, only: %i[show edit update] do delete 'scores', action: :destroy_scores, controller: :feedbacks, on: :member 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 890cf57a33..03b2b4015d 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.80.6", diff --git a/test/controllers/score_items_controller_test.rb b/test/controllers/score_items_controller_test.rb index 781fcc97e7..525add3b70 100644 --- a/test/controllers/score_items_controller_test.rb +++ b/test/controllers/score_items_controller_test.rb @@ -8,10 +8,11 @@ def setup 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 + 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], @@ -19,57 +20,25 @@ def setup [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: { + patch evaluation_score_item_path(@evaluation, score_item, format: :json), params: { score_item: { - name: 'Test', - description: 'Test', + description: 'After 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 + + assert_response expected sign_out user if user.present? end end - test 'should update score item if course administrator' do + test 'should update all score items if course administrator' do exercise = @evaluation.evaluation_exercises.first - score_item = create :score_item, evaluation_exercise: exercise, - description: 'Before test', - maximum: '10.0' + score_items = create_list :score_item, 3, evaluation_exercise: exercise, + description: 'Before test', + maximum: '10.0' [ [@staff_member, :success], @@ -79,14 +48,33 @@ def setup [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' - } + patch evaluation_evaluation_exercise_score_items_path(@evaluation, exercise, format: :json), params: { + 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_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_equal 3, exercise.score_items.count + end + sign_out user if user.present? 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 b0b620409e..8bb3c4fa65 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1734,6 +1734,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" @@ -6162,6 +6167,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"