BitcreditProtocol / E-Bill

Core for Bitcredit project.
https://www.bit.cr/
MIT License
12 stars 2 forks source link

115: upload files in bill #209

Closed zupzup closed 1 week ago

zupzup commented 2 weeks ago

This fixes #115 and adds the functionality for uploading files when issueing a bill, as well as to locally view these uploaded files.

Notes

Besides that, I restructured a few things and added tests where possible and transitioned the contacts API to use tokio::fs, as to not block on the async runtime.

codecov[bot] commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 53.06799% with 566 lines in your changes missing coverage. Please review.

Project coverage is 14.44%. Comparing base (be86fb9) to head (114defe). Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/dht/client.rs 1.23% 160 Missing :warning:
src/web/handlers/bill.rs 0.00% 135 Missing :warning:
src/persistence/bill.rs 1.44% 68 Missing :warning:
src/util/file.rs 56.25% 42 Missing :warning:
src/service/bill_service.rs 93.89% 28 Missing :warning:
src/service/mod.rs 0.00% 24 Missing :warning:
src/persistence/contact.rs 0.00% 20 Missing :warning:
src/blockchain/chain.rs 0.00% 19 Missing :warning:
src/dht/mod.rs 0.00% 14 Missing :warning:
src/main.rs 0.00% 12 Missing :warning:
... and 10 more
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #209 +/- ## ========================================== + Coverage 6.68% 14.44% +7.75% ========================================== Files 42 45 +3 Lines 8277 8871 +594 ========================================== + Hits 553 1281 +728 + Misses 7724 7590 -134 ```

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

mtbitcr commented 1 week ago

LGTM

zupzup commented 1 week ago

Wow this one is quite massive. I can not really follow how the file upload stuff works just from the diffs but impl. looks fine. The rest of the refactoring also looks very good (as already mentioned). A bit small chunks might be easier to chew 😁

Yes, you're absolutely right, this escalated a bit :see_no_evil: - sorry! And in retrospect it seems a lot clearer, where there might have been good lines to split this by and I'll take care to keep things smaller, also for my own sanity's sake. :sweat_smile:

This will become easier with more structure as well, as it's already easier to spot which parts of the system will have to be touched by a certain change with the changes we already did. :+1: