Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][MIG] fieldservice_isp_account #1126

Merged
merged 24 commits into from
Nov 17, 2023

Conversation

michelerusti
Copy link

@michelerusti michelerusti commented Nov 1, 2023

brian10048 and others added 22 commits November 1, 2023 16:14
- This change removes most of the invoicing logic from fieldservice_account and puts it into its own module
[IMP] Add Project + Task to View for Context
Currently translated at 100.0% (36 of 36 strings)

Translation: field-service-14.0/field-service-14.0-fieldservice_isp_account
Translate-URL: https://translation.odoo-community.org/projects/field-service-14-0/field-service-14-0-fieldservice_isp_account/pt_BR/
Currently translated at 100.0% (43 of 43 strings)

Translation: field-service-15.0/field-service-15.0-fieldservice_isp_account
Translate-URL: https://translation.odoo-community.org/projects/field-service-15-0/field-service-15-0-fieldservice_isp_account/es_AR/
@michelerusti michelerusti force-pushed the 16.0-mig-fieldservice_isp_account branch from 2b3087f to 4900fbf Compare November 1, 2023 16:17
@michelerusti michelerusti marked this pull request as ready for review November 2, 2023 14:57
@michelerusti michelerusti force-pushed the 16.0-mig-fieldservice_isp_account branch 3 times, most recently from 58cf24f to 1ab1c87 Compare November 2, 2023 15:17
@michelerusti michelerusti mentioned this pull request Nov 2, 2023
48 tasks
@michelerusti michelerusti force-pushed the 16.0-mig-fieldservice_isp_account branch 2 times, most recently from 84ba63c to 68f2032 Compare November 6, 2023 15:09
@michelerusti michelerusti force-pushed the 16.0-mig-fieldservice_isp_account branch 3 times, most recently from 4f7ed96 to 043bdb3 Compare November 13, 2023 11:20
@michelerusti michelerusti force-pushed the 16.0-mig-fieldservice_isp_account branch 3 times, most recently from aa450f3 to 93f7cf6 Compare November 13, 2023 16:16
@michelerusti
Copy link
Author

michelerusti commented Nov 13, 2023

There are a bunch of errors regarding wrong INSERT and not-null constraint like this one:
https://github.com/OCA/field-service/actions/runs/6851350407/job/18627432083?pr=1126#step:8:628
psycopg2.errors.NotNullViolation: null value in column "sale_line_warn" violates not-null constraint DETAIL: ....

After some analysis, I conclude that those are not actually module related errors but are environment-caused. Correct me if I'm wrong please:
The failing tests are (taking the link above as an example) in fieldservice_project that errors out for the missing, required field, customer_id that is defined in fieldservice_account_analytic in this PR and not at all related to isp_account and that shouldn't fail in any case since the owner_id is passed as a value in the create
This is already suspect since those errors are not present in that PR's tests

After a bit of trial and error, I managed to have the expected result, meaning all the errors are related to the module of this PR

How?
Not installing fieldservice_account_analytic in the database initialization stage but in the "Run test" stage
By doing so, the tests pass correctly and I've fixed all the one that required to

Why?
idk, somehow if installed before, the database structure changes accordingly (hence the db errors) but the model and the python code is not loaded correctly. I haven't found out why

As per this PR, all the failing tests right now are to be considered junk.

@max3903 @brian10048 do you have any consideration about this? Am I right in some way or I'm doing something wrong?
Thanks

@brian10048 brian10048 added this to the 16.0 milestone Nov 13, 2023
@michelerusti michelerusti force-pushed the 16.0-mig-fieldservice_isp_account branch from 93f7cf6 to 96db93c Compare November 15, 2023 15:54
Copy link

@stevech091 stevech091 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functional TEST Ok

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@brian10048
Copy link
Contributor

/ocabot merge nobump

1 similar comment
@brian10048
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-1126-by-brian10048-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 525f59a into OCA:16.0 Nov 17, 2023
4 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 085edb0. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.