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

Improvements for "PDF document data" metabox #757

Closed
wants to merge 46 commits into from
Closed

Improvements for "PDF document data" metabox #757

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
72c030f
Move PDF document data markup to views
MohamadNateqi Mar 28, 2024
69e9b3d
WPCS
MohamadNateqi Mar 28, 2024
34ee8d1
Update and improvements for metabox markup
MohamadNateqi Mar 28, 2024
e39996f
Add auto update documents data using heartbeat
MohamadNateqi Mar 28, 2024
0058f6f
Generate minified JS and CSS files
MohamadNateqi Mar 28, 2024
ac6ec0d
Fix nonce validation
MohamadNateqi Apr 3, 2024
6ecfebe
Validate the document type
MohamadNateqi Apr 3, 2024
2251d16
Improve performance
MohamadNateqi Apr 3, 2024
00c299c
Better sanitize
MohamadNateqi Apr 3, 2024
ed6bc11
WPCS
MohamadNateqi Apr 3, 2024
7fd96b7
Add DocBlock
MohamadNateqi Apr 3, 2024
75c411b
Improvement
MohamadNateqi Apr 3, 2024
acc87cb
Fix bugs
MohamadNateqi Apr 3, 2024
2b66b21
Indentation fixes
alexmigf Apr 3, 2024
46ee270
Improve the meta box method to print all documents data,
MohamadNateqi Apr 9, 2024
bcbb368
Update version
MohamadNateqi Apr 9, 2024
969f928
Merge branch 'main' into document-data-metabox
alexmigf Apr 10, 2024
69e812e
Rename a parameter
MohamadNateqi Apr 10, 2024
146180c
Fix HPOS compatibility
MohamadNateqi Apr 10, 2024
0d02eda
Fix input width issue on Firefox
MohamadNateqi Apr 10, 2024
fedccf6
Merge branch 'main' into document-data-metabox
MohamadNateqi Apr 10, 2024
b407d25
Merge remote-tracking branch 'origin/document-data-metabox' into docu…
MohamadNateqi Apr 10, 2024
8fa23bf
Generate minified JS and CSS files
MohamadNateqi Apr 10, 2024
ab89e8c
Fix a bug
MohamadNateqi Apr 10, 2024
d19708a
Rename a parameter
MohamadNateqi Apr 12, 2024
bcccb26
Fix not removing dependent document
MohamadNateqi Apr 12, 2024
f9cbcca
Check if we have an order ID
MohamadNateqi Apr 12, 2024
8f7c72c
Better name for a hook
MohamadNateqi Apr 12, 2024
db3c341
Use another AS method
MohamadNateqi Apr 12, 2024
46e8aa9
Generate minified JS and CSS files
MohamadNateqi Apr 12, 2024
a13139e
Merge branch 'main' into document-data-metabox
MohamadNateqi Apr 12, 2024
813e746
Add document number to output
MohamadNateqi Apr 15, 2024
e424202
Revert outputting all documents in the meta box
MohamadNateqi Apr 15, 2024
721ef11
Merge branch 'main' into document-data-metabox
MohamadNateqi Apr 15, 2024
a58ff0f
Improvements for the default data labels
MohamadNateqi Apr 15, 2024
df58528
WPCS
MohamadNateqi Apr 16, 2024
0bbec3f
Improvement
MohamadNateqi Apr 16, 2024
55b47a8
Update the usage of a filter
MohamadNateqi Apr 16, 2024
a6c5927
Introducing a new propery in Order_Document class to store prerequisi…
MohamadNateqi Apr 16, 2024
48d0371
Check document prerequisites in `is_allowed()` method
MohamadNateqi Apr 17, 2024
2a9d4e9
Introduce the new `do_prerequisites_exist()` method
MohamadNateqi Apr 17, 2024
2393848
Remove an unnecessary filter
MohamadNateqi Apr 17, 2024
8c6ed82
Update `pdf_actions_meta_box()` method
MohamadNateqi Apr 17, 2024
c391c99
Updates
MohamadNateqi Apr 17, 2024
6e4349d
Merge branch 'main' into document-data-metabox
MohamadNateqi Apr 17, 2024
08cff80
Revert changes and improvements
MohamadNateqi Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 40 additions & 35 deletions includes/class-wcpdf-admin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,41 +564,43 @@ public function pdf_actions_meta_box( $post_or_order_object ) {
$documents = WPO_WCPDF()->documents->get_documents();

foreach ( $documents as $document ) {
$document_title = $document->get_title();
$document = wcpdf_get_document( $document->get_type(), $order );

if ( $document ) {
$document_url = WPO_WCPDF()->endpoint->get_document_link( $order, $document->get_type() );
$document_title = is_callable( array( $document, 'get_title' ) ) ? $document->get_title() : $document_title;
$document_exists = is_callable( array( $document, 'exists' ) ) ? $document->exists() : false;
$document_printed = $document_exists && is_callable( array( $document, 'printed' ) ) ? $document->printed() : false;
$document_printed_data = $document_exists && $document_printed && is_callable( array( $document, 'get_printed_data' ) ) ? $document->get_printed_data() : [];
$document_settings = get_option( 'wpo_wcpdf_documents_settings_'.$document->get_type() ); // $document-settings might be not updated with the last settings
$unmark_printed_url = ! empty( $document_printed_data ) && isset( $document_settings['unmark_printed'] ) ? WPO_WCPDF()->endpoint->get_document_printed_link( 'unmark', $order, $document->get_type() ) : false;
$manually_mark_printed = WPO_WCPDF()->main->document_can_be_manually_marked_printed( $document );
$mark_printed_url = $manually_mark_printed ? WPO_WCPDF()->endpoint->get_document_printed_link( 'mark', $order, $document->get_type() ) : false;
$class = [ $document->get_type() ];

if ( $document_exists ) {
$class[] = 'exists';
}
if ( $document_printed ) {
$class[] = 'printed';
}

$meta_box_actions[$document->get_type()] = array(
'url' => esc_url( $document_url ),
'alt' => "PDF " . $document_title,
'title' => "PDF " . $document_title,
'exists' => $document_exists,
'printed' => $document_printed,
'printed_data' => $document_printed_data,
'unmark_printed_url' => $unmark_printed_url,
'manually_mark_printed' => $manually_mark_printed,
'mark_printed_url' => $mark_printed_url,
'class' => apply_filters( 'wpo_wcpdf_action_button_class', implode( ' ', $class ), $document ),
);
$document = wcpdf_get_document( $document->get_type(), $order );

if ( ! $document || ( ! $document->exists() && ! $document->do_prerequisites_exist() ) ) {
continue;
}

$document_title = $document->get_title();
$document_url = WPO_WCPDF()->endpoint->get_document_link( $order, $document->get_type() );
$document_title = is_callable( array( $document, 'get_title' ) ) ? $document->get_title() : $document_title;
$document_exists = is_callable( array( $document, 'exists' ) ) ? $document->exists() : false;
$document_printed = $document_exists && is_callable( array( $document, 'printed' ) ) ? $document->printed() : false;
$document_printed_data = $document_exists && $document_printed && is_callable( array( $document, 'get_printed_data' ) ) ? $document->get_printed_data() : [];
$document_settings = get_option( 'wpo_wcpdf_documents_settings_'.$document->get_type() ); // $document-settings might be not updated with the last settings
$unmark_printed_url = ! empty( $document_printed_data ) && isset( $document_settings['unmark_printed'] ) ? WPO_WCPDF()->endpoint->get_document_printed_link( 'unmark', $order, $document->get_type() ) : false;
$manually_mark_printed = WPO_WCPDF()->main->document_can_be_manually_marked_printed( $document );
$mark_printed_url = $manually_mark_printed ? WPO_WCPDF()->endpoint->get_document_printed_link( 'mark', $order, $document->get_type() ) : false;
$class = [ $document->get_type() ];

if ( $document_exists ) {
$class[] = 'exists';
}
if ( $document_printed ) {
$class[] = 'printed';
}

$meta_box_actions[ $document->get_type() ] = array(
'url' => esc_url( $document_url ),
'alt' => "PDF " . $document_title,
'title' => "PDF " . $document_title,
'exists' => $document_exists,
'printed' => $document_printed,
'printed_data' => $document_printed_data,
'unmark_printed_url' => $unmark_printed_url,
'manually_mark_printed' => $manually_mark_printed,
'mark_printed_url' => $mark_printed_url,
'class' => apply_filters( 'wpo_wcpdf_action_button_class', implode( ' ', $class ), $document ),
);
}

$meta_box_actions = apply_filters( 'wpo_wcpdf_meta_box_actions', $meta_box_actions, $order->get_id() );
Expand Down Expand Up @@ -717,6 +719,9 @@ public function data_input_box_content( $post_or_order_object ) {

$document_types = apply_filters( 'wpo_wcpdf_meta_box_documents', array( 'invoice', 'packing-slip' ), $order );

// Allow getting documents even if their prerequisite documents doesn't exist.
add_filter( 'wpo_wcpdf_document_do_prerequisites_exist', '__return_true' );

foreach ( $document_types as $document_type ) {
$document = wcpdf_get_document( $document_type, $order );
if ( $document ) {
Expand Down Expand Up @@ -1039,7 +1044,7 @@ public function ajax_crud_document() {

try {
// Allow getting documents even if their prerequisite documents doesn't exist.
add_filter( 'wpo_wcpdf_prerequisite_documents', '__return_empty_array()' );
add_filter( 'wpo_wcpdf_document_do_prerequisites_exist', '__return_true' );
Copy link
Member

Choose a reason for hiding this comment

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

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()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

@alexmigf alexmigf Apr 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.


$document = wcpdf_get_document( $document_type, wc_get_order( $order_id ) );

Expand Down
2 changes: 1 addition & 1 deletion includes/class-wcpdf-frontend.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public function generate_document_shortcode( $atts, $content = null, $shortcode_

$document = wcpdf_get_document( $values['document_type'], $order );

if ( ! $document || ! $document->is_allowed() ) {
if ( ! $document || ! $document->is_allowed() || ! $document->do_prerequisites_exist() ) {
return;
}

Expand Down
12 changes: 9 additions & 3 deletions includes/documents/abstract-wcpdf-order-document.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,16 +535,22 @@ public function is_allowed(): bool {
}
}

// Check document prerequisites
return apply_filters( 'wpo_wcpdf_document_is_allowed', $allowed, $this );
}

public function do_prerequisites_exist(): bool {
MohamadNateqi marked this conversation as resolved.
Show resolved Hide resolved
$prerequisites_exist = true;

foreach ( $this->get_prerequisite_documents() as $prerequisite_document_type ) {
$prerequisite_document = wcpdf_get_document( $prerequisite_document_type, $this->order );

if ( $prerequisite_document && ! $prerequisite_document->exists() ) {
$allowed = false;
$prerequisites_exist = false;
break;
}
}

return apply_filters( 'wpo_wcpdf_document_is_allowed', $allowed, $this );
return apply_filters( 'wpo_wcpdf_document_do_prerequisites_exist', $prerequisites_exist, $this );
}

public function exists() {
Expand Down
4 changes: 2 additions & 2 deletions includes/wcpdf-functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function wcpdf_get_document( $document_type, $order, $init = false ) {
do_action( 'wpo_wcpdf_process_template_order', $document_type, $order->get_id() );
$document = WPO_WCPDF()->documents->get_document( $document_type, $order );

if ( ! $document || ! is_callable( array( $document, 'is_allowed' ) ) || ! $document->is_allowed() ) {
if ( ! $document || ! $document->is_allowed() || ! $document->do_prerequisites_exist() ) {
return apply_filters( 'wcpdf_get_document', false, $document_type, $order, $init );
}

Expand Down Expand Up @@ -79,7 +79,7 @@ function wcpdf_get_document( $document_type, $order, $init = false ) {

$document = WPO_WCPDF()->documents->get_document( $document_type, $order );

if ( ! $document || ! $document->is_allowed() ) {
if ( ! $document || ! $document->is_allowed() || ! $document->do_prerequisites_exist() ) {
return apply_filters( 'wcpdf_get_document', false, $document_type, $order, $init );
}

Expand Down
Loading