Improvements for "PDF document data" metabox

[MohamadNateqi]

close #756
close #760

Note: For the Pro extension compatibility, this branch should be used: https://github.com/wpovernight/woocommerce-pdf-ips-pro/pull/392.

---

This PR improves the way document data are displayed in the metabox. It includes a check for pending jobs and displays a message if the document is still in the queue.

It also uses WordPress heartbeat to update document data via AJAX, eliminating the need to refresh the page. However, for this to work correctly with Pro documents, the Pro plugin requires some improvements that have been done in the PR mentioned above.

For test purposes, the below snippet can be used to stop the queue from running jobs:

function wpo_disable_default_runner() {
	if ( class_exists( 'ActionScheduler' ) ) {
		remove_action( 'action_scheduler_run_queue', array( ActionScheduler::runner(), 'run' ) );
	}
}

add_action( 'init', 'wpo_disable_default_runner', 10 );

Then, the job can be manually run to test the AJAX update.

---

This PR also fixes the issue that prevents displaying a dependent document if it already exists. If the invoice document is a requirement for another document (such as the credit note), removing the invoice from the "PDF document data" meta box will prevent the other dependent document from being displayed. For example, when a credit note or receipt document exists, removing the invoice prevents displaying those documents, even though they've already been generated.

This issue happens on both the AJAX request and page refresh/load (AJAX request also gets the whole page content the same as page load and refreshes the meta box with new content) because the invoice is a requirement for those documents, and the is_allow() method checks for that and returns false when it cannot find an invoice.

A new filter has been added to address this issue, which needs this filter in the Pro extension.

[alexmigf]

For some reason I can't delete the Receipt:

It may not seem directly relevant, but it could be worth investigating.

The time inputs width is strange:

[alexmigf]

Maybe we should uniform the number and date with the third party documents too?

[MohamadNateqi]

Maybe we should uniform the number and date with the third party documents too?

@alexmigf Thank you for mentioning this. Fixed here: a58ff0f.

[alexmigf]

I'm having the "Packing Slip" duplicated, not sure why.

[MohamadNateqi]

I'm having the "Packing Slip" duplicated, not sure why.

Could you please check if your Pro extension is on the related branch?

[alexmigf]

It's in main.

[MohamadNateqi]

It should be on the pdf-document-data branch, since as I mentioned here, I moved packing slip from the Pro extension to the Free plugin.

[alexmigf]

Ok, I didn't saw that comment, I will check that again.

[alexmigf]

Seems to work fine now regarding the Packing Slip.

[MohamadNateqi]

@alexmigf Maybe it's better to have these lines in the free plugin:
https://github.com/wpovernight/woocommerce-pdf-ips-pro/pull/392/files#diff-1aaf16af4dbfbd03af9365d3d5500e1e5e75a84b1e49aa025a71eb5b38875578R1354-R1358

What do you think?

[alexmigf]

@MohamadNateqi yes, I agree.

[alexmigf]

Maybe something like this would work below line 1049? Because we are not forcing the document generation at this stage, so it's safe to call the document first.

if ( false === apply_filters( 'wpo_wcpdf_bypass_prerequisites_restriction', true, 'ajax_crud_document', $document ) && ! empty( $document->get_prerequisite_documents() ) ) {
	return;
}

This can be also applied to the data_input_box_content() inside the loop I think.

And maybe inside is_allowed()?

[MohamadNateqi]

We can add checking prerequisites inside the is_allowed() method, inside the condition you mentioned, but again we need to change the value that the wpo_wcpdf_bypass_prerequisites_restriction filter returns, before line 1049, since otherwise wcpdf_get_document() method won't return the document.
So, I think it's again adding a filter above the line 1049.

[MohamadNateqi]

In this commit: 08cff80, I added again checking prerequisites to is_allowed() method, but made it false as default, since otherwise it'll allow creation of documents without prerequisites, and also used the filter you mentioned.
Please let me know what you think.

[alexmigf]

You could avoid having it inside is_allowed(), and call it when you need to check the is_allowed() output. I think returning an empty array seems strange to me and a bit like hacking.

[MohamadNateqi]

If we don't add checking prerequisites to the is_allowed() method, the wcpdf_get_document() method will return a document even if the prerequisites of it don't exist. so I thought maybe it should be inside that as default?

I think returning an empty array seems strange to me and a bit like hacking.

You're right.
In the recent commit, I used the filter you mentioned: wpo_wcpdf_bypass_prerequisites_restriction. What do you think about it?

[MohamadNateqi]

Since a document object is required to call the do_prerequisites_exist(), to check the prerequisite before forcing the document generation, I must call the wcpdf_get_document() again without the third parameter. I'm not sure if this is a good approach.

[alexmigf]

You can get the document from here: $document = WPO_WCPDF()->documents->get_document( $document_type, $order );

[alexmigf]

If you add to is_allowed() I think you don't need to add on generate_document_ajax()? I haven't tested.

[MohamadNateqi]

I added that to is_allowed() method like this:

if ( ! $this->do_prerequisites_exist( $context ) ) {
	return false;
}

But since it'll bypass by default, it won't affect it. If we change the bypass to false by default, then we need to use the filter to change it anywhere we want, for example, for metabox.

I think checking prerequisites is a requirement for the wcpdf_get_document() method, so maybe it's better to keep the bypass false as default?

Also, we can use the WPO_WCPDF()->documents->get_document() method here, but I think it'll make the usage of wcpdf_get_document() and it should be something that should be considered for using of wcpdf_get_document() method.

If you agree, I will revert the related change to this matter and open another PR to address this issue, which is related to issue #760.

[alexmigf]

If you agree, I will revert the related change to this matter and open another PR to address this issue, which is related to issue #760.

Yes, it's better, we continue from there.

[MohamadNateqi]

I'm closing this PR to open new PRs to address the above issues separately.