mastodon icon indicating copy to clipboard operation
mastodon copied to clipboard

Add `API::Errors` concern for API controllers and add i18n values for strings

Open mjankowski opened this issue 2 years ago • 6 comments

Changes to the Api::BaseController error handling:

  • Some of the error classes handled by the controller were not in the spec, added them
  • Added a check to the spec which asserts against the json error value in the response (previously only http response code was checked)
  • Move the strings out of the controller class and into i18n yml
  • Move the API error handling methods to an Api::ErrorHandling concern

mjankowski avatar Nov 17 '23 14:11 mjankowski

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 84.89%. Comparing base (86500e3) to head (4e63e64). Report is 278 commits behind head on main.

:exclamation: Current head 4e63e64 differs from pull request most recent head c533a02. Consider uploading reports for the commit c533a02 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #27925      +/-   ##
==========================================
- Coverage   85.01%   84.89%   -0.13%     
==========================================
  Files        1059     1059              
  Lines       28277    28376      +99     
  Branches     4538     4551      +13     
==========================================
+ Hits        24040    24090      +50     
- Misses       3074     3124      +50     
+ Partials     1163     1162       -1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Nov 17 '23 15:11 codecov[bot]

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions[bot] avatar Nov 30 '23 16:11 github-actions[bot]

This pull request has resolved merge conflicts and is ready for review.

github-actions[bot] avatar Nov 30 '23 17:11 github-actions[bot]

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions[bot] avatar Jan 24 '24 11:01 github-actions[bot]

Rebased after https://github.com/mastodon/mastodon/pull/28864 added the coverage for missing classes.

This one is now basically two things: moving the error handling to concern, adding i18n strings. If we only want one of these, but not the other (or want them in sepsearate PRs), let me know.

mjankowski avatar Jan 24 '24 11:01 mjankowski

This pull request has resolved merge conflicts and is ready for review.

github-actions[bot] avatar Jan 24 '24 12:01 github-actions[bot]

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions[bot] avatar Mar 14 '24 10:03 github-actions[bot]

Rebased this after errors concern merged, so is this now just a) the i18n stuff, b) helper methods for that, c) spec improvement to check response body in addition to code.

I think that spec stuff is helpful, but am less certain about the i18n stuff. Will pull out spec change into separate PR, and then probably close this and drop remainder.

mjankowski avatar Mar 14 '24 13:03 mjankowski

Closing in favor of linked PR.

mjankowski avatar Mar 14 '24 13:03 mjankowski