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.

Sign up for GitHub

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

Jump to bottom

[16.0][IMP] l10n_es_aeat_mod349: implement more error texts #3331

Merged
merged 1 commit into from
Apr 19, 2024
Merged

[16.0][IMP] l10n_es_aeat_mod349: implement more error texts #3331

merged 1 commit into from
Apr 19, 2024

Conversation

ACheung-FactorLibre
Copy link
Contributor

Implement more error texts when:

  • Country Code is not found in VAT
  • Country Code is not from Europe
  • Total Operation Amount is negative

More unit tests have been added.

@OCA-git-bot
Copy link
Contributor

Hi @pedrobaeza,
some modules you are maintaining are being modified, check this out!

@ACheung-FactorLibre ACheung-FactorLibre marked this pull request as ready for review November 30, 2023 11:23
@pedrobaeza pedrobaeza added this to the 16.0 milestone Nov 30, 2023
pedrobaeza
pedrobaeza reviewed Dec 11, 2023
View reviewed changes
l10n_es_aeat_mod349/models/mod349.py Outdated
country_code = record.partner_vat[0:2]
if country_code.isnumeric():
country_code = record.country_id.code
european_codes = record.partner_id._get_aeat_europe_codes()
Copy link
Member

Choose a reason for hiding this comment

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

Esto no haría falta obtenerlo si no hay código de país. Tampoco entiendo el doble código para los códigos de países de Europa. ¿Por qué es necesario?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Listo, lo he cambiado para que lo obtenga cuando sí exista código de país. Realmente no es duplicado, ya que si no tiene código, no va a verificar si existe dentro de los códigos europeos.

@ACheung-FactorLibre ACheung-FactorLibre force-pushed the 16.0-imp-l10n_es_aeat_mod349 branch 3 times, most recently from 201b940 to b37fc7d Compare December 12, 2023 11:13
@ACheung-FactorLibre ACheung-FactorLibre force-pushed the 16.0-imp-l10n_es_aeat_mod349 branch 2 times, most recently from 897cd22 to d389284 Compare January 10, 2024 07:22
@RodrigoBM
Copy link
Contributor

Buenas @pedrobaeza, @ACheung-FactorLibre te ha realizado los cambios requeridos, es posible mergear?

@HaraldPanten HaraldPanten mentioned this pull request Apr 15, 2024
Merged
@HaraldPanten
Copy link
Contributor

HaraldPanten commented Apr 15, 2024 •
edited
Loading

@RodrigoBM podéis hacer rebase para ver si el runboat se construye correctamente?

Gracias.

@ACheung-FactorLibre
Copy link
Contributor Author

@RodrigoBM podéis hacer rebase para ver si el runboat se construye correctamente?

Gracias.

Buenas @HaraldPanten,

Listo, gracias.

HaraldPanten
HaraldPanten approved these changes Apr 16, 2024
View reviewed changes
Copy link
Contributor

@HaraldPanten HaraldPanten left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Probados los 3 casos en runboat:

  • NIF sin código de país.
  • País no encontrado en el grupo de países de Europa.
  • Importe negativo del período.

Revisión funcional 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). 🤖

@HaraldPanten
Copy link
Contributor

@pedrobaeza Si técnicamente te encaja, fusionamos. Lo tendremos en cuenta para la migración a V17 que estamos llevando a cabo.

@RodrigoBM
Copy link
Contributor

ping @pedrobaeza

pedrobaeza
pedrobaeza approved these changes Apr 19, 2024
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-3331-by-pedrobaeza-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit b1c2ddb into OCA:16.0 Apr 19, 2024
5 checks passed
@OCA-git-bot
Copy link
Contributor

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

@ACheung-FactorLibre ACheung-FactorLibre deleted the 16.0-imp-l10n_es_aeat_mod349 branch April 22, 2024 06:31
@ACheung-FactorLibre ACheung-FactorLibre mentioned this pull request Apr 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SoniaViciana SoniaViciana SoniaViciana left review comments

@HaraldPanten HaraldPanten HaraldPanten approved these changes

@aritzolea aritzolea aritzolea approved these changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
7 participants
@ACheung-FactorLibre @OCA-git-bot @RodrigoBM @HaraldPanten @pedrobaeza @aritzolea @SoniaViciana