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

New: adds UBL as additional output format for documents #521

Merged
merged 131 commits into from
Sep 14, 2023

Conversation

alexmigf
Copy link
Member

@alexmigf alexmigf commented Jun 2, 2023

closes #481

This PR adds support for UBL format in companion with the existing PDF output. This will replace our UBL addon, and future developments on UBL must be done on this plugin instead.

Specs

Screenshots

Document settings for UBL format:

Screenshot from 2023-06-02 14-17-37

UBL output preview:

Screenshot from 2023-06-02 14-21-45

Taxes classification:

Screenshot from 2023-06-02 14-18-10

New dynamic action button (can be generated for other UBL documents):

Screenshot from 2023-06-02 14-18-37

Bulk actions support:

Screenshot from 2023-06-02 14-19-37

New independent meta box for UBL in the order page:

Screenshot from 2023-06-02 14-20-04

UBL document settings export/import support:

Screenshot from 2023-06-05 11-59-49

Copy link
Contributor

@MohamadNateqi MohamadNateqi left a comment

Choose a reason for hiding this comment

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

Since you have applied WPCS on the UBL directory, it would be great if you could rename the function to follow the sneak case, or we can create another issue as the changes may be significant.

ubl/Handlers/Ubl/PaymentMeansHandler.php Outdated Show resolved Hide resolved
ubl/Collections/Collection.php Outdated Show resolved Hide resolved
ubl/Documents/Document.php Outdated Show resolved Hide resolved
ubl/Documents/UblDocument.php Outdated Show resolved Hide resolved
ubl/Settings/TaxesSettings.php Outdated Show resolved Hide resolved
includes/class-wcpdf-admin.php Outdated Show resolved Hide resolved
includes/class-wcpdf-admin.php Outdated Show resolved Hide resolved
includes/class-wcpdf-admin.php Outdated Show resolved Hide resolved
includes/wcpdf-functions.php Outdated Show resolved Hide resolved
includes/wcpdf-functions.php Outdated Show resolved Hide resolved
@alexmigf alexmigf requested a review from MohamadNateqi August 28, 2023 13:18
Copy link
Contributor

@MohamadNateqi MohamadNateqi left a comment

Choose a reason for hiding this comment

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

Great work, Alex!
Thank you for your efforts.

Copy link
Contributor

@dwalkerpriv dwalkerpriv left a comment

Choose a reason for hiding this comment

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

Works fine on my end. Settings exported and imported without issues as well

@alexmigf
Copy link
Member Author

alexmigf commented Sep 6, 2023

I was thinking that maybe we should rename the plugin following this implementation. Joe have mentioned this in past meetings, maybe something like this:

  • PDF & UBL Invoices & Packing Slips for WooCommerce

But I don't like seeing two & in the title, maybe we should apply a / or a ,?

What do you guys think?

@dwalkerpriv @YordanSoares @MohamadNateqi @Terminator-Barbapapa

@MohamadNateqi
Copy link
Contributor

@alexmigf I agree with you that having two "&" symbols can look cluttered.
I think "PDF/UBL Invoices & Packing Slips for WooCommerce" seems good.

Also, I wanted to say that I like the idea of renaming, as it improves clarity and reflects this new feature that can also be good in terms of SEO, but I'm also sort of worried about the impact of this change as this plugin has been around for years with this name.

@alexmigf
Copy link
Member Author

alexmigf commented Sep 6, 2023

but I'm also sort of worried about the impact of this change as this plugin has been around for years with this name.

Shouldn't have a huge impact, we have also renamed it recently to change the "for WooCommerce" to the end of the name.

I think we should have some space outside the / because it terms of SEO this reads like this:

  • PDF/UBL => PDFUBL
  • PDF / UBL => PDF UBL

The second seems better to me.

@MohamadNateqi
Copy link
Contributor

Yeah, having some space around / is better.

@YordanSoares
Copy link
Contributor

This makes me recall that in Spain people often joke about how good is our plugin, but how difficult is to remember its name.

To be honest, if we're willing to rename the plugin, I would prefer to run a brainstorming and look for a branding short name, not a name that states-all-the-document-types-our-plugin-offers.

Then we can mention what our plugin does as a subtitle or within the plugin descriptions in both on WordPress.org and our store. For instance:

  • On WordPress.org:
    • NewPluginName - PDF / UBL Invoices & Packing Slips for WooCommerce
  • On our store, the new extension names would be shorted and easy to remember and differentiate (our customer often buy a different plugin because I think they get mess with the long names):
    • NewPluginName - Professional
    • NewPluginName - Premium Templates

Here, some naming ideas to start with:

  • EasyInvoicing
  • QuickInvoice
  • FastInvoicing
  • InvoicingWise
  • WooInvoicing

@alexmigf
Copy link
Member Author

alexmigf commented Sep 6, 2023

By rename I didn't mean change all name, just a way to include the UBL there somehow. Full rename, or brand rename should be discussed privately in Slack I think, and maybe including the marketing department from UpdraftPlus.

@dwalkerpriv
Copy link
Contributor

In regards to incorporating UBL in the name, "PDF / UBL Invoices & Packing Slips for WooCommerce" is pretty simple and straight forward enough.

ubl/Documents/UblDocument.php Outdated Show resolved Hide resolved
includes/settings/class-wcpdf-settings-ubl.php Outdated Show resolved Hide resolved
includes/settings/class-wcpdf-settings-ubl.php Outdated Show resolved Hide resolved
Copy link
Contributor

@Terminator-Barbapapa Terminator-Barbapapa left a comment

Choose a reason for hiding this comment

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

I think we are there. Awesome work! 🎊

@alexmigf alexmigf merged commit d864f83 into main Sep 14, 2023
5 checks passed
@alexmigf alexmigf deleted the 481-ubl-integration branch September 14, 2023 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate UBL code from extension
5 participants