RDISCROWD-5567 enhance task guidelines images #801

n00rsy · 2023-01-12T16:56:08Z

Enhance Image Uploading for GIGwork Project Instructions
Enable uploading task guidelines images to cloud storage

coveralls · 2023-01-12T17:28:12Z

Pull Request Test Coverage Report for Build 3959929636

35 of 35 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.01%) to 93.97%

Totals
Change from base Build 3766833203:	0.01%
Covered Lines:	16456
Relevant Lines:	17512

💛 - Coveralls

kbecker42

The update to the themes pointer (static/..., templates/..., etc) would probably need to be done after the templates PR has been merged. This way, this PR can point to themes in the main branch.

kbecker42

This looks good!

I'm wondering if we should consider avoiding increasing the size of the repo with the large binary image (14.8MB) in the unit test, with two options:

Use a smaller image that is just over 5MB in size to trigger the error condition.
Skip the large image upload and instead refactor the file size calculation into a separate method and mock this method to return True in the unit test, thus triggering the error condition without actually uploading a large image. preferable

In this case, the large image could then be deleted from the commit. Thank you.

Adding @XiChenn for comment as well.

pybossa/view/projects.py

test/test_web.py

pybossa/view/projects.py

XiChenn · 2023-01-13T22:04:32Z

This looks good!

I'm wondering if we should consider avoiding increasing the size of the repo with the large binary image (14.8MB) in the unit test, with two options:

Use a smaller image that is just over 5MB in size to trigger the error condition.

Skip the large image upload and instead refactor the file size calculation into a separate method and mock this method to return True in the unit test, thus triggering the error condition without actually uploading a large image. preferable

In this case, the large image could then be deleted from the commit. Thank you.

Adding @XiChenn for comment as well.

I'm with Kory not to upload large files to the repo. How about using a a small file (~10k) for the "test_task_presenter_image_upload" and using Mock as Kory suggested to test this function "upload_task_guidelines_image" specifically with mock of large file? Some reference with "mock" could be found here.

XiChenn · 2023-01-13T22:13:14Z

pybossa/view/projects.py

+        file_size_mb = file.seek(0, os.SEEK_END) / 1024 / 1024
+        file.seek(0, os.SEEK_SET)
+        file.filename = secure_filename(file.filename)
+        if file_size_mb < 5:


How about putting the magic number "5" to the configuration (e.g. settings_local.py)? It will not only be easier for future changes, but also the unit test could be beneficial from it (you can configure a much smaller value for the unit test). It will add some one-time complexity for the code deployment. At minimum, we can consider use a const variable.

XiChenn · 2023-01-13T22:13:30Z

pybossa/view/projects.py

+                current_app.config.get('AVATAR_ABSOLUTE')
+            ))
+        else:
+            flash(gettext('File must be smaller than 5 MB.'))


Same here for the magic number.

kbecker42 · 2023-01-17T19:25:01Z

pybossa/view/projects.py

+        file_size_mb = file.seek(0, os.SEEK_END) / 1024 / 1024
+        file.seek(0, os.SEEK_SET)
+        file.filename = secure_filename(file.filename)
+        if file_size_mb < current_app.config.get('MAX_IMAGE_UPLOAD_SIZE_MB'):


It would be a good idea to also include a default value in case a dev does not have this setting. Otherwise, if the setting is missing, the upload will always fail.

current_app.config.get('MAX_IMAGE_UPLOAD_SIZE_MB', 5)

kbecker42 · 2023-01-17T19:26:50Z

pybossa/view/projects.py

+                current_app.config.get('AVATAR_ABSOLUTE')
+            ))
+        else:
+            flash(gettext('File must be smaller than ' + str(current_app.config.get('MAX_IMAGE_UPLOAD_SIZE_MB')) + ' MB.'))


current_app.config.get('MAX_IMAGE_UPLOAD_SIZE_MB', 5)

XiChenn

great job! Thank you

nsyed22 added 5 commits January 11, 2023 10:28

added upload_task_guidelines_image route

f87937f

add security checks

ef56a06

add errors to response

9c5d4c4

implement uploading and file size check

1bbdef5

remove test flash

729e9ef

kbecker42 reviewed Jan 12, 2023

View reviewed changes

n00rsy changed the title ~~Rdiscrowd 5567 enhance task guidelines images~~ RDISCROWD-5567 enhance task guidelines images Jan 12, 2023

n00rsy mentioned this pull request Jan 12, 2023

RDISCROWD-5567 enhance task guidelines images bloomberg/pybossa-default-theme#386

Merged

nsyed22 added 3 commits January 13, 2023 11:49

add file size tests

5888129

fix typo in test description

9f49341

added multiple file uploads test

c57b92e

kbecker42 reviewed Jan 13, 2023

View reviewed changes

pybossa/view/projects.py Outdated Show resolved Hide resolved

test/test_web.py Outdated Show resolved Hide resolved

pybossa/view/projects.py Outdated Show resolved Hide resolved

kbecker42 requested a review from XiChenn January 13, 2023 19:08

nsyed22 added 2 commits January 13, 2023 15:07

use send 413 response code on large file

b4ced2e

refactor errors to error

02d2ff1

XiChenn reviewed Jan 13, 2023

View reviewed changes

nsyed22 and others added 6 commits January 17, 2023 10:09

switch to small images

23183f3

remove added files

ede0754

Delete setuplogins.py

cb7f384

use magic number for max image upload size

8e1aa8a

edit settings_local template

a75dc39

edit settings_test template

daefff8

kbecker42 reviewed Jan 17, 2023

View reviewed changes

nsyed22 added 3 commits January 17, 2023 14:35

use default value for MAX_IMAGE_UPLOAD_SIZE_MB

f622f6c

initial push for edge case tests

719c94a

refactor and fix edge cases test

f5548b1

n00rsy marked this pull request as ready for review January 19, 2023 15:27

bump theme SHA

9653bfc

XiChenn approved these changes Jan 19, 2023

View reviewed changes

n00rsy merged commit 9400f8e into main Jan 19, 2023

This was referenced Jan 19, 2023

Revert "RDISCROWD-5567 enhance task guidelines images" #802

Closed

RDISCROWD-5567 Enhance task guidelines images #803

Merged

XiChenn deleted the RDISCROWD-5567-enhance-task-guidelines-images branch February 8, 2023 14:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RDISCROWD-5567 enhance task guidelines images #801

RDISCROWD-5567 enhance task guidelines images #801

n00rsy commented Jan 12, 2023 •

edited

Loading

coveralls commented Jan 12, 2023 •

edited

Loading

kbecker42 left a comment

kbecker42 left a comment •

edited

Loading

XiChenn commented Jan 13, 2023

XiChenn Jan 13, 2023 •

edited

Loading

XiChenn Jan 13, 2023

kbecker42 Jan 17, 2023 •

edited

Loading

kbecker42 Jan 17, 2023

XiChenn left a comment

RDISCROWD-5567 enhance task guidelines images #801

RDISCROWD-5567 enhance task guidelines images #801

Conversation

n00rsy commented Jan 12, 2023 • edited Loading

coveralls commented Jan 12, 2023 • edited Loading

Pull Request Test Coverage Report for Build 3959929636

💛 - Coveralls

kbecker42 left a comment

Choose a reason for hiding this comment

kbecker42 left a comment • edited Loading

Choose a reason for hiding this comment

XiChenn commented Jan 13, 2023

XiChenn Jan 13, 2023 • edited Loading

Choose a reason for hiding this comment

XiChenn Jan 13, 2023

Choose a reason for hiding this comment

kbecker42 Jan 17, 2023 • edited Loading

Choose a reason for hiding this comment

kbecker42 Jan 17, 2023

Choose a reason for hiding this comment

XiChenn left a comment

Choose a reason for hiding this comment

n00rsy commented Jan 12, 2023 •

edited

Loading

coveralls commented Jan 12, 2023 •

edited

Loading

kbecker42 left a comment •

edited

Loading

XiChenn Jan 13, 2023 •

edited

Loading

kbecker42 Jan 17, 2023 •

edited

Loading