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

[IMP] internal_transfer_with_agreed_amount: modify write vals in one method i#24002 #1631

Conversation

CLaurelB
Copy link
Contributor

@CLaurelB CLaurelB commented Nov 2, 2023

Modify in a single method the values that will be written in the account
move lines so that the method can be inherited in case it is necessary
to modify more values in the lines.

In addition, two methods are added to pre and post process payments, to
be used if modifications are required in an inheritance.

Finally, obtaining currencies related to the internal transfer have been
moved to a separate method to facilitate the inheritance of the method
that validates the number of currencies.

Changelog:

  • Set debit and credit values to write in _prepare_values_from_lines
    method when the exchange currency is the same as the company currency.
  • Add methods to be inherited when payments also needs modifications.
  • Get the currencies in a new method named _get_currencies.

@CLaurelB CLaurelB force-pushed the 16.0-set_all_lines_vals_in_one_method-CLaurelB branch 2 times, most recently from 2cd1258 to 3e8fc96 Compare November 3, 2023 03:43
@CLaurelB
Copy link
Contributor Author

CLaurelB commented Nov 3, 2023

@rolandojduartem could you review, please?

vals = {"debit": line.debit} if line.amount_currency > 0 else {"credit": line.credit}
sign = 1 if line.amount_currency > 0 else -1
vals.update({"amount_currency": sign * self.agreed_amount, "currency_id": currency.id})
return vals

def _postprocessing_payment_with_aggred_amount(self, payment):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def _postprocessing_payment_with_aggred_amount(self, payment):
def _postprocessing_payment_with_agreed_amount(self, payment):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

vals = self._prepare_values_from_lines(debit, is_same_company_currency=True)
debit.with_context(check_move_validity=False).write(vals)
vals = self._prepare_values_from_lines(credit, is_same_company_currency=True)
credit.with_context(check_move_validity=False).write(vals)
amls.move_id.action_post()
to_reconcile.reconcile()
return
Copy link
Contributor

@rolandojduartem rolandojduartem Nov 3, 2023

Choose a reason for hiding this comment

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

Sorry, you have to call the method before the return too self._postprocessing_payment_with_agreed_amount(payment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

@CLaurelB CLaurelB force-pushed the 16.0-set_all_lines_vals_in_one_method-CLaurelB branch from 3e8fc96 to f08b8d4 Compare November 3, 2023 16:16
@CLaurelB
Copy link
Contributor Author

CLaurelB commented Nov 3, 2023

@rolandojduartem could you review again, please?

@CLaurelB CLaurelB force-pushed the 16.0-set_all_lines_vals_in_one_method-CLaurelB branch 3 times, most recently from 0782a2b to 207c450 Compare November 3, 2023 20:49
Copy link
Contributor

@rolandojduartem rolandojduartem left a comment

Choose a reason for hiding this comment

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

LGTM 👍 please, @luisg123v, could you review it?

@luisg123v luisg123v self-requested a review November 3, 2023 21:31
Copy link
Contributor

@luisg123v luisg123v left a comment

Choose a reason for hiding this comment

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

By reading commit message, it's not clear why the change, there's only a changelog.

@luisg123v
Copy link
Contributor

Missing task ID on MR title

@CLaurelB CLaurelB force-pushed the 16.0-set_all_lines_vals_in_one_method-CLaurelB branch from 207c450 to 79e19c7 Compare November 5, 2023 07:54
@CLaurelB CLaurelB changed the title [IMP] internal_transfer_with_agreed_amount: modify write vals in one method [IMP] internal_transfer_with_agreed_amount: modify write vals in one method i#24002 Nov 5, 2023
@CLaurelB
Copy link
Contributor Author

CLaurelB commented Nov 5, 2023

Missing task ID on MR title

Since it's related to a ticket, I added the ticket ID on the title.

@CLaurelB
Copy link
Contributor Author

CLaurelB commented Nov 5, 2023

@luisg123v could you review again, please?

@CLaurelB CLaurelB requested a review from luisg123v November 5, 2023 09:34
@CLaurelB CLaurelB force-pushed the 16.0-set_all_lines_vals_in_one_method-CLaurelB branch from 79e19c7 to c88971d Compare November 6, 2023 17:22
…method

Modify in a single method the values that will be written in the account
move lines so that the method can be inherited in case it is necessary
to modify more values in the lines.

In addition, two methods are added to pre and post process payments, to
be used if modifications are required in an inheritance.

Finally, obtaining currencies related to the internal transfer have been
moved to a separate method to facilitate the inheritance of the method
that validates the number of currencies.

Changelog:
- Set debit and credit values to write in _prepare_values_from_lines
  method when the exchange currency is the same as the company currency.
- Add methods to be inherited when payments also needs modifications.
- Get the currencies in a new method named _get_currencies.
@CLaurelB CLaurelB force-pushed the 16.0-set_all_lines_vals_in_one_method-CLaurelB branch from c88971d to 7bda48e Compare November 6, 2023 17:47
Copy link
Contributor

@luisg123v luisg123v left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@luisg123v luisg123v merged commit d9d4b7c into Vauxoo:16.0 Nov 7, 2023
3 checks passed
@luisg123v luisg123v deleted the 16.0-set_all_lines_vals_in_one_method-CLaurelB branch November 7, 2023 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants