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

Qual - Optimize requests in sells journal for situation invoices #30609

Conversation

lvessiller-opendsi
Copy link
Contributor

Qual - Optimize requests in sells journal for situation invoices

  • not reload "Facture" object when already loaded from SQL request
  • add a method "getPrevProgressFromInvoiceCycleRefAndType" to determine the progress percent from "cycle ref" and invoice "type"

Remark
When you got a lot of invoices and you set "INVOICE_USE_SITUATION" to 1, you can excess the max execution time.

@aspangaro
Copy link
Member

aspangaro commented Aug 13, 2024

Hi @lvessiller-opendsi

Have you tried your code with the constant INVOICE_USE_SITUATION = 2, which adds "progressive" situation invoices to v20?

@eldy eldy added PR waiting more user feedbacks We are waiting feedback of someone or more testers to validate this PR Discussion Some questions or discussions are opened and wait answers of author or other people to be processed and removed PR waiting more user feedbacks We are waiting feedback of someone or more testers to validate this PR labels Aug 13, 2024
@lvessiller-opendsi
Copy link
Contributor Author

Hi @lvessiller-opendsi

Have you tried your code with the constant INVOICE_USE_SITUATION = 2, which adds "progressive" situation invoices to v20?

Hi @aspangaro, I haven't tried with INVOICE_USE_SITUATION = 2 yet.
I did'nt know it.
What is the difference with "INVOICE_USE_SITUATION = 1" ?

@aspangaro
Copy link
Member

Hi @lvessiller-opendsi

It's a new mode for situation invoice integrated in v20 (hidden for the moment). The mode 2 of the constant must be replaced the mode 1 officialy in v21.

This work comes from the GIFF situation invoice, the difference between the two modes is that situation invoices were on a cumulative mode and now it will be on a progressive mode.

@lvessiller-opendsi
Copy link
Contributor Author

Hi @lvessiller-opendsi

It's a new mode for situation invoice integrated in v20 (hidden for the moment). The mode 2 of the constant must be replaced the mode 1 officialy in v21.

This work comes from the GIFF situation invoice, the difference between the two modes is that situation invoices were on a cumulative mode and now it will be on a progressive mode.

Hello,

This PR not depends on INVOICE_USE_SITUATION const, I only add a method not to fetch and load invoice object (many SQL requests).
I used the value got early in the SQL request to retrieve the previous situation percent.
It's only to optimize and avoid an excessive execution time.

@@ -6764,56 +6764,23 @@ public function get_prev_progress($invoiceid, $include_credit_note = true)
// phpcs:enable
global $invoicecache;

if (is_null($this->fk_prev_id) || empty($this->fk_prev_id) || $this->fk_prev_id == "") {
Copy link
Member

@eldy eldy Aug 14, 2024

Choose a reason for hiding this comment

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

Why removing this ?
We know that if this condition is true, we must do nothing and we can return 0 (no fetch at all to do)

W1W1-M and others added 4 commits September 6, 2024 13:49
* Moved invoice line class to separate file. Removed unnecessary requires.

* Fixed requires

* Fixed require
* fix phpstan

* fix phpstan
@lvessiller-opendsi
Copy link
Contributor Author

I had some troubles in retrieving the "develop" branch and some unwanted PR was introduced in it.

@lvessiller-opendsi lvessiller-opendsi deleted the new-sellsjournal-get-prev-progress branch September 6, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Some questions or discussions are opened and wait answers of author or other people to be processed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants