AnalogIO / coffeecard_app

Cross-platform coffee card app for Cafe Analog
https://www.cafeanalog.dk/app
MIT License
6 stars 1 forks source link

Removed http_utils #418

Closed TTA777 closed 1 year ago

TTA777 commented 1 year ago

Closes #417 Closes #387

Its functionality has already been replaced by ServerFailure, and is now causing an issue where we are accidentaly calling it with just a string, which then triggers an exception

marfavi commented 1 year ago

Since we're closing #387, does this mean that the backend now "maps errors correctly"? Or do we need a new issue on this?

TTA777 commented 1 year ago

Honestly, I'm unsure what @fremartini filed the original issue on specifically. If I were to guess, I would say it was in response to the OpenApi spec does not expose the error object that it can return on all endpoints, which results in us having to decode it manually, instead of using a generated method like we do for all other DTOs.

If my assumption is correct, I would say we should file a new issue for updating the ServerFailure once the OpenApi spec exposes the error object, and just close #387 now

fremartini commented 1 year ago

This issue was filed because when the user attempts to log in with an incorrect passcode, they would see the json object {'error': 'message'} so we would have to decode it. If the linked closed issues)prs have solved this or if the openapi spec has been updated this issue can be closed. Can someone verify this?

codecov[bot] commented 1 year ago

Codecov Report

Merging #418 (1abc265) into develop (a20186a) will decrease coverage by 0.11%. The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #418      +/-   ##
===========================================
- Coverage    47.42%   47.32%   -0.11%     
===========================================
  Files           85       84       -1     
  Lines         1478     1475       -3     
===========================================
- Hits           701      698       -3     
  Misses         777      777              
Flag Coverage Δ
unittests 47.32% <100.00%> (-0.11%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/cubits/login/login_cubit.dart 77.27% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

TTA777 commented 1 year ago

The OpenApi spec has not been updated yet, however all requests go through the executor now. If a request fails in the executor, we manually decode it in ServerFailure.fromResponse and return the ServerFailure, so the loginCubit would never get the raw json in the new implementation.

Whether we choose to close the issue, or update it, so it pertains to Serverfailure instead, is fine with me.