-
Notifications
You must be signed in to change notification settings - Fork 0
qtype: Abgabe von Dateien #210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: wysiwig
Are you sure you want to change the base?
Conversation
e8aa39b
to
7cd6300
Compare
question.php
Outdated
public function get_expected_data(): array|string { | ||
return [ | ||
constants::QT_VAR_RESPONSE => PARAM_RAW_TRIMMED, | ||
constants::QT_VAR_DRAFT_AREAS => PARAM_RAW_TRIMMED, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das ist nicht ganz ideal, weil die draft area ids dann auch zu jedem Step in der Datenbank abgespeichert werden, obwohl diese Information später gar keinen Nutzen mehr hat. In ISIS ist question_attempt_step_data unsere mit Abstand größte Tabelle. Ich würde daher eine andere Lösung bevorzugen.
Mir fallen ein:
- Wir greifen selber auf die Daten via
optional_param
zu. Die Lösung ist nicht perfekt, weil\question_usage_by_activity::process_all_actions
eigentlich auch ein optionales Argument$postdata
hat, das aber nur für Testzwecke gedacht ist. Sollte also hinnehmbar sein. - Wir speichern die Info in der User-Session, z.B. unter
$SESSION->qtype_questionpy_draft_areas[$attemptid]
. Du kannst in diese globale Variable einfach reinschreiben. Nachteil ist, dass man den Attempt dann nicht in mehreren Tabs öffnen und einen beliebigen davon speichern kann!?
Was denkst du?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich glaube de Nachteil von 2. ist schon ein Blocker... Ich würde morgen eher 1. umsetzen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im Behaviour: questionpy-org/moodle-qbehaviour_questionpy@6691384
Hier habe ich einfach den großen Commit amended.
classes/constants.php
Outdated
/** @var string */ | ||
public const FILEAREA_ATTEMPT_FILES = 'response_files'; | ||
/** @var string */ | ||
public const QT_VAR_ATTEMPT_FILES = '_files'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Der Unterstrich wird uns Probleme bereiten. Beim Regrading mit \question_attempt::regrade
wird \question_attempt_step::get_submitted_data
aufgerufen und das überspringt "cached data", also Felder beginnend mit Unterstrich.
Ich verstehe, dass du das gemacht hast, damit im Behaviour set_qt_var
benutzt werden kann. Das macht es jetzt richtig blöd, oder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Es hat mir physische Schmerzen bereitet und ich habe die Edge-Cases nicht getestet, aber hier wäre ein Commit, der das Problem löst: questionpy-org/moodle-qbehaviour_questionpy@4b3b22c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wie besprochen jetzt anders: questionpy-org/moodle-qbehaviour_questionpy@6691384
d9808e0
to
67d012a
Compare
67d012a
to
a1b52a3
Compare
return dom_utils::html_to_fragment($this->doc, $html); | ||
} | ||
|
||
private function render_readonly(question_attempt $qa, question_ui_renderer $renderer): DOMNode { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hier wäre es besser, die Moodle-Renderer-Logik zu benutzen (z.B. als weitere Methode im bestehenden Renderer, ggf. mit einem Template), damit Themes die Möglichkeit zu Anpassungen haben. Dann können wir diese Ansicht auch in assignsubmission_qpy praktischerweise verwenden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das verstehe ich nicht ganz. question_ui_renderer
ist doch gar kein Renderer im Moodle-Sinne bzw. er implementiert renderer_base
nicht
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So meinte ich das auch nicht. Das Erzeugen des HTMLs für die Dateiliste und die dazugehörige Logik sollte aber besser in qtype_questionpy_renderer
liegen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mir ist gerade noch eine Sache durch den Kopf gegangen. Die Bezeichnung "Attempt File" (u.a. |
5de7eff
to
62be6ca
Compare
public function get_limits_in(context $context): validatable_upload_limits { | ||
global $CFG, $PAGE; | ||
|
||
$maxfiles = $this->element->getAttribute('max_files'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Für die XML-Attribute sollten wir wahrscheinlich besser Bindestriche benutzen, oder?
Siehe auch questionpy-org/moodle-qbehaviour_questionpy#4.
Um "etwas mehr" von dem Standard-Moodle-Code (insb.
question_file_saver
) wiedererwenden zu können, passiert das speichern von abgegebenen Dateien in zwei Schritten:qbehaviour_questionpy::process_action
werden alle Draft-Areas, die Teil der Abgabe sind (Mitgeliefert durch ein Hidden Input außerhalb des Iframes), in eine Draft-Area vereint, und dort unter/<url-encodeter Upload-Feld-Name>/
abgelegt. Dann wird einsubdirs_question_file_saver
(der normale macht keine Unterordner) erstellt.subdirs_question_file_saver::save_files
auf, und speichert den ganzen Inhalt der Draft-Area in die "permanente" File-Area. Als value wird in den Step-Daten ein Hash über alle Dateien (oder''
falls keine hochgeladen wurden) abgelegt. Der wird später (u.A.?) genutzt, um zu prüfen, ob sich der Step vom vorherigen unterscheidet.Beim Laden wird in
question_ui_renderer
direkt aus der persistenten File-Area in eine Draft-Area pro File-Upload kopiert.form_filemanager
-Rendering hat Nebeneffekte ($PAGE->requires
), daher mussquestion_ui_renderer::render
jetzt immer im Iframe aufgerufen werden (war auch vorher so, aber ich denke nur durch Zufall, ich bin zwischendurch in diese Falle gelaufen.)TODOs
qpy_response
schreiben (nachträgliches Setzen von normalen QT-Vars ist nicht vorgesehen). Das müsste dann vorm API-Call vereint werden.question_ui_metadata_extractor
das machen, oder wir setzen beim ersten rendern eine QT-Var...min-height
-Hack ist ein Hack. Die Größe eines ggf. offenen Modals wird nicht in die Iframe-scrollHeight
einberechnet, deshalb ist der File-Manager ohne das quasi unbenutzbar. Mittelfristig denke ich, diemin-height
sollte automagisch nur gesetzt werden, wenn tatsächlich ein File-Upload benutzt wird. Langfristig wäre es dann ideal, eine Möglichkeit zu finden, das Modal außerhalb des Iframes anzuzeigen.file_prepare_draft_area
(oderquestion_attempt::prepare_response_files_draft_itemid
) aufrufen, da passieren magische und potenziell wichtige Hacks bzgl.$filerecord->source
.