fix: rework error handling for add_category

[pcarles]

Hello !

This PR fixes #253.
I think that we can keep the unique constraint in the DB on top of this change as proposed by @josecelano in the original issue

Following this change, I think that we should move some of the error logic that is inside the databases crate to the services one.
All of these errors should not be raised at database level but rather at the service level:

• database::Error::UsernameTaken
• database::Error::EmailTaken
• database::Error::TagAlreadyExists
• database::Error::TorrentAlreadyExists
• database::Error::TorrentTitleAlreadyExists

This will permit to avoid using database error message parsing which is implementation specific and error prone (like here or here)

I'd be happy to open another PR to rework the listed errors if you aggree with this new approach
That's my first contribution here, and I am also pretty new to the Rust ecosystem so don't hesitate to tell me if anything's wrong

[codecov-commenter]

Codecov Report

Merging #376 (5dc0017) into develop (41c80f3) will decrease coverage by 0.29%.
The diff coverage is 62.50%.

@@             Coverage Diff             @@
##           develop     #376      +/-   ##
===========================================
- Coverage    42.34%   42.05%   -0.29%     
===========================================
  Files           79       79              
  Lines         4822     4829       +7     
===========================================
- Hits          2042     2031      -11     
- Misses        2780     2798      +18     

Files	Coverage Δ
src/databases/database.rs	30.55% <ø> (ø)
src/errors.rs	26.78% <ø> (-3.02%)	⬇️
...s/from_v1_0_0_to_v2_0_0/databases/sqlite_v2_0_0.rs	76.81% <ø> (-4.35%)	⬇️
src/databases/mysql.rs	0.00% <0.00%> (ø)
src/databases/sqlite.rs	22.12% <0.00%> (-0.89%)	⬇️
src/services/category.rs	81.63% <83.33%> (-0.98%)	⬇️

... and 2 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

[pcarles]

Force pushed to add signed commit

[josecelano]

ACK 5dc0017

[josecelano]

Hi @pcarles, thank you very much for your contribution. It looks good to me! I think this a very good refactor to decouple the domain from the database, and It will make it easier to implement new database drivers like PostgreSQL in the future.

[pcarles]

Hello @josecelano
Perfect! I'll open another PR during the week to refactor the other DB errors I mentioned.

What is the preferred workflow here? Should I open an issue before the PR, or just pushing a PR explaining the changes is enough

[josecelano]

Hello @josecelano Perfect! I'll open another PR during the week to refactor the other DB errors I mentioned.

What is the preferred workflow here? Should I open an issue before the PR, or just pushing a PR explaining the changes is enough

We have a preference for opening new issues. You can get feedback even before starting working on the PR. Otherwise, we do not know you are working on it. In fact, in this case, if you are planning to open more than one issue/PR I would suggest opening a parent issue like this to track the state of the whole refactor.

In general these are my "good practices" (not the repo or org rules):

• Open an issue always (with a good explanation: Why, Context, Links, Related issues, ...)
• Track epic issues (big issues with small subtasks)
• Open a draft PR as soon as possible
• We usually review "draft" PRs but we know they can change totally or be discarded
• Make issues as PR as small as possible
• Small steps and parallel changes
• Try to add tests

In fact, this is something I want to discuss in our next meeting.