-
Notifications
You must be signed in to change notification settings - Fork 303
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
Quiz exercises
: Simplify drag-and-drop image upload
#7259
Conversation
*/ | ||
private URI saveDragAndDropImage(Path basePath, MultipartFile file, @Nullable Long questionId) throws IOException { | ||
Path savePath = fileService.generateFilePath("dnd_image_", FilenameUtils.getExtension(file.getOriginalFilename()), basePath); | ||
FileUtils.copyToFile(file.getInputStream(), savePath.toFile()); |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression
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.
The DND quiz preview breaks for me with draggable image items. The video shows the current behavior on the left (develop
, running locally) and the behavior with this PR on the right (on TS4). Note that the image already seems to break while dragging it.
screenshot-2023-09-26_001986.mp4
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.
I tested uploading files a bit and was able to consistently reproduce a bug that occurs, when I edit a quiz, stay on the page.
- Upload at least one file
- Press "Save"
- Delete any (could be the one you uploaded could be another file that previously was uploaded)
- Try to save the changes
Desktop.2023.09.27.-.00.19.28.06.mp4
8ccb845
to
c211afd
Compare
@pal03377 Fixed the your first issue in d8d3e22, the second in 052d639 @Kroko-fant Fixed in c211afd (not yet ready for a retest, need to fix something else first) |
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
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.
Tested in testing session - quizzes worked as expected
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.
Tested in testing session - Multiple choice / Single choice and "Fill in the blanks" exercises work as expected.
Re-evalution works as expected.
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.
Tested in testing session, works as expected
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.
tested on ts1, all previusly found issues are now resolved. code lgtm.
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.
I created a quiz with a question (DnD). Works perfect.
However, I created another quiz in the same course, and when I look for the question (the recently created one) to import into this new quiz, it does not appear in the list.
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.
Code looks good 👍
Checklist
General
Server
Client
Motivation and Context
Follow-up of #5733 and #5427 to replace the file upload before entity persistence with a multipart file and entity upload.
Reimplementation after #6456 got reverted. After implementing the file service, the original issue got resolved.
Description
The changes only affect the drag-and-drop section of the quizzes. But because the quiz editor allows creating all different types of quiz questions, I had to make the component saving the entity to retrieve the files from potential drag-and-drop question children and upload them together with the files.
It affects the Creation, Update, Re-evaluation and Import of quiz exercises. Additionally, it also affects the creation of drag-and-drop mode quizzes. However, those use the same creation endpoint.
Steps for Testing
We need to fully test the quiz functionalities, including:
During all of these five checks, it makes sense to work
Always check if the quiz gets rendered as expected.
Exam Mode Testing
Most of the changes are unrelated to the exam mode. I do not think testing is necessary to test the exam mode as extensively. However, it makes sense to just test the normal quiz exam workflow (creation and conduction) to be sure.
The only change that does require testing here is the import functionality of an existing exercise.
Review Progress
I squashed all changes from #6456 that already got approved, so technically, it should be enough to review the rest.
Code Review
Manual Tests
Exam Mode Tests
Test Coverage
Screenshots
Upload background before (before changes):
Upload Background before (after changes):
Upload background after (before changes):
Upload background after (after changes):
Upload picture drag item after (before changes):
Upload picture drag item after (after changes):