open-contracting / credere-backend

A tool that facilitates the participation of Micro, Small, and Medium businesses (MSMEs) in the Colombian public procurement market.
https://credere.readthedocs.io
BSD 3-Clause "New" or "Revised" License
5 stars 0 forks source link

427 external onboarding #433

Closed yolile closed 1 month ago

yolile commented 1 month ago

closes #427 closes #375

yolile commented 1 month ago

@jpmckinney I will add some tests and the translations after your review

coveralls commented 1 month ago

Pull Request Test Coverage Report for Build 11410466320

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
app/dependencies.py 2 3 66.67%
app/mail.py 1 3 33.33%
<!-- Total: 14 17 82.35% -->
Totals Coverage Status
Change from base Build 11386165049: -0.1%
Covered Lines: 1748
Relevant Lines: 2033

💛 - Coveralls
yolile commented 1 month ago

I think there is no more logic associated with additional_comments right? If so, we can remove that from frontend and backend.

I think so, yes, will remove it

jpmckinney commented 1 month ago

Not sure if you saw my question:

What does the incoming JSON look like? Is the field just blank (empty string) or is it missing?

In any case, can you try with

from pydantic import condecimal

disbursed_final_amount: condecimal(gt=0)

Edit: I think the issue is that when not using "strict" mode then Pydantic does coercion, which in this case is assigning 0. However, since we are parsing JSON, I am not sure if setting strict=True on the model/field will just cause new issues; gt=0 seems the safest option. https://docs.pydantic.dev/latest/concepts/strict_mode/

yolile commented 1 month ago

What does the incoming JSON look like? Is the field just blank (empty string) or is it missing?

I'm writing the tests and Pydantic is indeed working as expected so it seems the front end is sending 0 as the default value.

condecimal(gt=0)

I tested it now and this covers all cases, so I will use that instead. Could you remind me how we translate pydantic error messages?

yolile commented 1 month ago

Never mind, I will add the validation and message in the front end

The validation was excluded in the front end because https://github.com/open-contracting/credere-backend/pull/433#discussion_r1806415839