Skip to content

throwing correct error message whenever a user tries to insert a filefrom ocis into moodle that is bigger than the configured max size. #118

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

throwing correct error message whenever a user tries to insert a filefrom ocis into moodle that is bigger than the configured max size. #118

wants to merge 1 commit into from

Conversation

@nirajacharya2
Copy link
Contributor

@nirajacharya2 nirajacharya2 commented Aug 6, 2025 •
edited
Loading

related to: #117

@nirajacharya2 nirajacharya2 self-assigned this Aug 6, 2025
@nirajacharya2 nirajacharya2 force-pushed the fix-uplod-error-max-size branch 2 times, most recently from bf54f71 to 28996ca Compare August 6, 2025 10:51
@individual-it individual-it mentioned this pull request Aug 7, 2025
Open
@nirajacharya2 nirajacharya2 force-pushed the fix-uplod-error-max-size branch 2 times, most recently from 26018b4 to 2b4eaab Compare August 13, 2025 04:19
@nirajacharya2 nirajacharya2 marked this pull request as ready for review August 13, 2025 04:42
@individual-it
Copy link
Member

individual-it commented Aug 13, 2025

@PhMemmel could you try that

@PhMemmel
Copy link

PhMemmel commented Aug 23, 2025

Hi @individual-it , sorry for the delay. Yes, will test and give feedback.

@PhMemmel
Copy link

PhMemmel commented Aug 25, 2025

I tested on our testing instance. Same error still occurs...

@nirajacharya2 nirajacharya2 force-pushed the fix-uplod-error-max-size branch 9 times, most recently from 34cfcc0 to 4f1384c Compare August 27, 2025 08:40
@nirajacharya2
throwing correct error message whenever a user tries to insert a file…
7c2e3c6 
… from ocis into moodle that is bigger than the configured max size.
@nirajacharya2 nirajacharya2 force-pushed the fix-uplod-error-max-size branch from 4f1384c to 7c2e3c6 Compare August 27, 2025 08:56
@nirajacharya2
Copy link
Contributor Author

nirajacharya2 commented Aug 28, 2025

I tested on our testing instance. Same error still occurs...

can you test again?

@nirajacharya2
Copy link
Contributor Author

nirajacharya2 commented Aug 28, 2025

@individual-it i have added an ui test just to check if it actually works. i don't think it should be merged. ill remove it after

@nirajacharya2
Copy link
Contributor Author

nirajacharya2 commented Sep 8, 2025

@PhMemmel can you please test it again? i have added ui tests to verify this works. just need your approval now.

@PhMemmel
Copy link

PhMemmel commented Sep 8, 2025

Will retest, but will probably take a little bit of time.

@PhMemmel
Copy link

PhMemmel commented Sep 23, 2025 •
edited
Loading

Hi, sorry for the long delay. I retested again and the error still happens. I'm trying to add a file test.txt that I just generated by using dd if=/dev/zero of=test.txt bs=1M count=600 on an instance that has a moodle upload limit of 512MB configured.

image

phil-davis
phil-davis reviewed Sep 23, 2025
View reviewed changes
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Just some minor things that I noticed.

tests/behat/maxUpload.feature
@@ -0,0 +1,26 @@
@max_uploas @javascript @repository_ocis @repository
Copy link
Contributor

@phil-davis phil-davis Sep 23, 2025

Choose a reason for hiding this comment

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

Suggested change
@max_uploas @javascript @repository_ocis @repository
@max_uploads @javascript @repository_ocis @repository

.drone.star
"update-ca-certificates",
"cd /var/www/html/moodle",
'vendor/bin/behat --config /var/www/behatdata/behatrun/behat/behat.yml --tags="@ocis"',
'vendor/bin/behat --config /var/www/behatdata/behatrun/behat/behat.yml --tags="@max_uploas"',
Copy link
Contributor

@phil-davis phil-davis Sep 23, 2025

Choose a reason for hiding this comment

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

Suggested change
'vendor/bin/behat --config /var/www/behatdata/behatrun/behat/behat.yml --tags="@max_uploas"',
'vendor/bin/behat --config /var/www/behatdata/behatrun/behat/behat.yml --tags="@max_uploads"',

tests/behat/behat_repository_ocis.php

/**
* Step definition for checking if error message is present on screen
* @When I should see error :errorMessage
Copy link
Contributor

@phil-davis phil-davis Sep 23, 2025

Choose a reason for hiding this comment

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

Suggested change
* @When I should see error :errorMessage
* @Then I should see error :errorMessage

This looks like a Then step.

@nirajacharya2
Copy link
Contributor Author

nirajacharya2 commented Sep 25, 2025

Hi, sorry for the long delay. I retested again and the error still happens. I'm trying to add a file test.txt that I just generated by using dd if=/dev/zero of=test.txt bs=1M count=600 on an instance that has a moodle upload limit of 512MB configured.
image

can you give me the trace of the error you are getting

@PhMemmel
Copy link

PhMemmel commented Sep 25, 2025

Stack trace unfortunately not really helpful:

JSON.parse: unexpected end of data at line 1 column 1 of the JSON data
Datei: https://MYHOST/theme/yui_combo.php?rollup/3.18.1/yui-moodlesimple.js
Zeile: 24447

@https://MYHOST/theme/yui_combo.php?rollup/3.18.1/yui-moodlesimple.js:24447:18
ln: 84
https://MYHOST/lib/javascript.
complete

ln: 35
https://MYHOST/theme/yui_combo
_notify

Not more information available from the network tab unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@phil-davis phil-davis phil-davis left review comments

@individual-it individual-it Awaiting requested review from individual-it

At least 1 approving review is required to merge this pull request.

Assignees

@nirajacharya2 nirajacharya2

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nirajacharya2 @individual-it @PhMemmel @phil-davis