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.

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

fix: 4219 - check if new picture is big enough before server upload #4224

Merged
merged 1 commit into from
Jun 24, 2023

Conversation

monsieurtanuki
Copy link
Contributor

What

  • On the server side, there are constraints on the size of the new pictures uploaded: at least width of 640 or height of 160.
  • If that condition is not fulfilled, the server returns an error.
  • Which is problematic for us, as we use background tasks
    • the user believes the image is uploaded (at least in the app)
    • the background task is doomed to fail forever at each retry
  • The solution is to check the size before sending any new image to the server.
  • This constraint does not seem to apply to existing image crop.

Screenshot

Screenshot_2023-06-22-19-59-53

Fixes bug(s)

Impacted files

  • app_en.arb: added 2 labels for a "too small image" dialog!
  • background_task_image.dart: now we check before server call if the image is big enough
  • crop_page.dart: now we check if the image is big enough before new image upload
  • pubspec.lock: wtf
  • pubspec.yaml: upgraded crop_image for controller bug fix

Impacted files:
* `app_en.arb`: added 2 labels for a "too small image" dialog!
* `background_task_image.dart`: now we check before server call if the image is big enough
* `crop_page.dart`: now we check if the image is big enough before new image upload
* `pubspec.lock`: wtf
* `pubspec.yaml`: upgraded crop_image for controller bug fix
@codecov-commenter
Copy link

Codecov Report

Merging #4224 (d14c21b) into develop (95c3a67) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #4224      +/-   ##
===========================================
- Coverage    11.00%   10.96%   -0.05%     
===========================================
  Files          282      282              
  Lines        14015    14069      +54     
===========================================
  Hits          1543     1543              
- Misses       12472    12526      +54     
Impacted Files Coverage Δ
...ooth_app/lib/background/background_task_image.dart 0.00% <0.00%> (ø)
packages/smooth_app/lib/pages/crop_page.dart 0.00% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

),
1 / cropConversionFactor,
);
switch (CropRotationExtension.fromDegrees(rotationDegrees)!) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI in that kind of situation a return switch from Dart 3 may look (a bit) better

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FYI in that kind of situation a return switch from Dart 3 may look (a bit) better

@g123k Fair enough!
I wouldn't say it looks better (just a matter of taste), but thank you for reminding me the new dart syntax!

@monsieurtanuki monsieurtanuki merged commit 57eff45 into openfoodfacts:develop Jun 24, 2023
6 checks passed
@monsieurtanuki
Copy link
Contributor Author

Thank you @g123k for your review!

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

Successfully merging this pull request may close these issues.

Small pictures cannot be uploaded, but we know it too late
3 participants