Skip to content
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.

Sign up for GitHub

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

Jump to bottom

File submission 403 #559

Closed
wants to merge 27 commits into from
Closed

File submission 403 #559

wants to merge 27 commits into from

Conversation

srobotta
Copy link

@srobotta srobotta commented Mar 25, 2024 •
edited
Loading

This pull request combines:

  • File upload question type: which is originally from Laurent David
  • Behat tests for that feature from Luca Bösch
  • Make it ready for Moodle 4.3 from Luca Bösch
  • Fixes of mine:
    • there can be more than one upload file question in a questionnary
    • the mandatory field check works as expected

@mchurchward
Copy link
Contributor

The pull request should just contain the code it deals with. And it needs to be rebased against the latest MOODLE_401_STABLE. And, what exactly does this address?

@srobotta
Copy link
Author

Hi Mike,

The code contains the new question type "file upload". We wanted/needed this feature. However, this is not yet part of the official release. The original work has been done by Laurent David, but when we tried out the code, we noticed a lot of issues. That's why I did changes based on his work. Apart from this, Luca maily wrote Behat tests to cover the new feature.
Before doing the pull request, I did merge the MOODLE_401_STABLE branch, so it should be up to date.

Best regards, Stephan

@mchurchward
Copy link
Contributor

Okay. Then you need to change this PR. PR's should only be for the feature they deal with. Otherwise it is too complicated to deal with. The commits that were for general unrelated fixes (such as Luca's CI and 4.3 fixes) should not be part of this.
Luca's changes are in a different PR.

@srobotta
Copy link
Author

srobotta commented Apr 5, 2024

Hi Mike,

The problem that I see here is that the original pull request #471 which we used and discovered that it was not correctly working. I myself have no knowledge of how I could extend this original pull request, therefore I took the changes, applied my fixes and created a new pull request. Sorry for the hazzle.
Lucas #542 deals with the 4.3 compatibility, the changes from him in this pull request acutally add Behat tests for the file type question.
The whole package can be tested at once without the need of having several pull request to test which patially would even fail because they depend on each other.
Therefore my question to you, how can we proceed here that both, you and me do not have much effort in applying the changes?

Best regards, Stephan

@srobotta srobotta force-pushed the file-submission-403 branch 2 times, most recently from a30fd38 to 570275e Compare April 5, 2024 09:40
mchurchward
mchurchward requested changes Aug 21, 2024
View reviewed changes
Copy link
Contributor

@mchurchward mchurchward left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long delay. This needs to be split up into multiple pull requests. Note that one of the PR's you reference from Luca has already been merged. I would suggest that you do a new PR off of the current MOODLE_401_STABLE branch that deals only with the File submission feature. Any other features/fixes need to likewise be in new PR's.

FelixDiLenarda and others added 20 commits August 21, 2024 16:06
@FelixDiLenarda
Change default of 'resp_view' select setting to never (PoetOS#550)
6253d1a
@lucaboesch @srobotta
Prepare for Moodle 4.3.
a906ac0
@lucaboesch @srobotta
Fixing Moodle Code Checker issues.
0861ca8
@laurentdavid @srobotta
Add file question type
6aa0ea8
@laurentdavid @srobotta
Fix preview submission (no sesskey)
763ab68
@laurentdavid @srobotta
Fix PHPdoc issue with pluginfile
19335bf
@laurentdavid @srobotta
Further fixes:
a067434 
* Fix file cs issues
* Fix behat test for file
@lucaboesch @srobotta
Tackle creation of dynamic properties.
71712af
@lucaboesch @srobotta
Update version to 2023101500.
c6bbfe2
@srobotta
Fix upload when several file upload questions are used.
604bf5c 
- Adjust siganture of methods to their parent
- Limit the files to upload to one for each question
- Use questionaire id for file draft area instead of question id in
  the questionaire.
@srobotta
Fix handling of file uploads
c4f93c5 
- allow upload of one file only.
- new submit does not load previously existing files.
@srobotta
Change refence of file.itemid to questionnaire_response_file.id
485bc50
@srobotta
update existing files.itemid to questionnaire_response_file.id
1df20f1
@lucaboesch @srobotta
Adding savepoint.
ba60a47
@lucaboesch @srobotta
Improvements to satisfy code checker.
5fe7dce
@srobotta
Make the file upload work with required setting, fix delete of records
de7e244
@laurentdavid @srobotta
Add file question type
3e23090
@laurentdavid @srobotta
Add file question type
7ac2fe4
@lucaboesch @srobotta
File question type Behat tests.
8d31fd5
@srobotta
Fix behat test with two file uploads
61e3a2c
@mchurchward
Copy link
Contributor

I may have broken your rebase. Can you do it one more time?

@srobotta
Copy link
Author

I have finished my work, and will create a new pull request.

@srobotta srobotta closed this Aug 28, 2024
@srobotta srobotta mentioned this pull request Aug 28, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mchurchward mchurchward mchurchward requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@srobotta @mchurchward @lucaboesch @Syxton @FelixDiLenarda @laurentdavid