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

Jump to bottom

UBL+ and PDF in modal windows #591

Closed
wants to merge 5 commits into from
Closed

UBL+ and PDF in modal windows #591

wants to merge 5 commits into from

Conversation

Verony-makesIT
Copy link
Contributor

@Verony-makesIT Verony-makesIT commented Feb 4, 2018 •
edited

Some preparations to see how contributing works before I commit my "Client UBL+" implementation.

  • Created a modal pdf viewer window for viewing Invoices instead of opening a tab window.
  • Added some UBL+ language strings.
    If something is not added in the right or correct way, please inform me how to do it just as it should.

Verony added 2 commits January 29, 2018 20:50
Added UBL+ lang strings
ab18048
Modal Pdf viewer in Invoices
b893c3f 
View your generated Pdf-Invoices in a modal window.
@Verony-makesIT Verony-makesIT reopened this Feb 4, 2018
Send invoice data as XML based on clients Zugferd/UBL+ version setting.
7dc630c 
Based on the individual XML version of your customer (Zugferd, UBL +), a PDF (+ zugferd?) or an extra (UBL) XML file is added to the e-mail.
All selectable e-invoicing XML capabilities are based on the correct xml templates.
Kovah
Kovah suggested changes Feb 9, 2018
View reviewed changes
Copy link
Contributor

@Kovah Kovah left a comment

Choose a reason for hiding this comment

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

Sadly, the whole pull request is a mess. You re-introduce bugs by reverting code.
That's why the contribution guide states that you should rebase your changes on top of the existing changes. This would have prevented this situation where I had to review for almost one hour and requesting you to change half of your code.

application/helpers/mailer_helper.php Outdated
@@ -5,7 +5,7 @@
* InvoicePlane
*
* @author InvoicePlane Developers & Contributors
* @copyright Copyright (c) 2012 - 2018 InvoicePlane.com
* @copyright Copyright (c) 2012 - 2017 InvoicePlane.com
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert this change

application/helpers/mpdf_helper.php Outdated
* @license https://invoiceplane.com/license.txt
* @link https://invoiceplane.com
* @author InvoicePlane Developers & Contributors
* @copyright Copyright (c) 2012 - 2017 InvoicePlane.com
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. Please revert

application/helpers/mpdf_helper.php Outdated
define('_MPDF_TEMP_PATH', UPLOADS_TEMP_MPDF_FOLDER);
define('_MPDF_TTFONTDATAPATH', UPLOADS_TEMP_MPDF_FOLDER);
define('_MPDF_TEMP_PATH', FCPATH . 'uploads/temp/mpdf/');
define('_MPDF_TTFONTDATAPATH', FCPATH . 'uploads/temp/mpdf/');
Copy link
Contributor

Choose a reason for hiding this comment

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

Same. Revert please.

application/helpers/mpdf_helper.php Outdated
@@ -72,32 +70,26 @@ function pdf_create(
}

// Check if the archive folder is available
if (!(is_dir(UPLOADS_ARCHIVE_FOLDER) || is_link(UPLOADS_ARCHIVE_FOLDER))) {
mkdir(UPLOADS_ARCHIVE_FOLDER, '0777');
if (!(is_dir('./uploads/archive/') || is_link('./uploads/archive/'))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same. Revert please.

application/helpers/mpdf_helper.php Outdated
// Watermark
if (get_setting('pdf_watermark')) {
$mpdf->showWatermarkText = true;
}

$mpdf->WriteHTML((string) $html);
$mpdf->WriteHTML($html);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same. Revert please.

application/modules/mailer/helpers/phpmailer_helper.php Outdated
@@ -31,11 +31,13 @@ function phpmail_send(
$bcc = null,
$more_attachments = null
) {
require_once(FCPATH . 'vendor/phpmailer/phpmailer/PHPMailerAutoload.php');
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert the whole file please!

application/modules/users/models/Mdl_users.php Outdated
* @license https://invoiceplane.com/license.txt
* @link https://invoiceplane.com
* @author InvoicePlane Developers & Contributors
* @copyright Copyright (c) 2012 - 2017 InvoicePlane.com
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert please

application/modules/users/models/Mdl_users.php Outdated
@@ -55,7 +55,7 @@ public function validation_rules()
'user_email' => array(
'field' => 'user_email',
'label' => trans('email'),
'rules' => 'required|valid_email|is_unique[ip_users.user_email]'
'rules' => 'required|callback_validate_email|is_unique[ip_users.user_email]'
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert please

application/modules/users/models/Mdl_users.php Outdated
@@ -147,7 +158,7 @@ public function validation_rules_existing()
'user_email' => array(
'field' => 'user_email',
'label' => trans('email'),
'rules' => 'required|valid_email'
'rules' => 'required|callback_validate_email'
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert please

application/modules/users/models/Mdl_users.php Outdated
* @param string $str
* @return bool
*/
public function validate_email($str)
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert please

Sven Vandevenne added 2 commits February 11, 2018 16:36
Revert "Modal Pdf viewer in Invoices"
6b67f00 
This reverts commit b893c3f.

# Conflicts:
#	application/modules/invoices/controllers/Ajax.php
#	application/modules/invoices/views/view.php
Revert "Send invoice data as XML based on clients Zugferd/UBL+ versio…
04f5a90 
…n setting."

This reverts commit 7dc630c.

# Conflicts:
#	application/modules/invoices/controllers/Ajax.php
#	application/modules/invoices/views/view.php
@Kovah
Copy link
Contributor

Kovah commented Apr 9, 2018

@Verony-12BaDev Could we re-do this pull request and split it up from the PDF modal feature? Would really like to implement the feature.

@Kovah Kovah changed the title V1.6.0 UBL+ and PDF in modal windows Apr 9, 2018
@Kovah
Copy link
Contributor

Kovah commented Jun 28, 2018

@Verony-12BaDev Could you do this pull request again on top of the current version?

@UnderDogg
Copy link
Contributor

Needs label : 1.6 or 1.5.11 or something. Needs Milestone?

@Kovah
Copy link
Contributor

Kovah commented Aug 12, 2018

It's not scheduled for a version yet.

@nielsdrost7
Copy link
Contributor

Issue https://development.invoiceplane.com/browse/IP-762 created. We either fix the pull-request or make a new one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kovah Kovah Kovah requested changes

Assignees
No one assigned
Labels
Change requested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Verony-makesIT @Kovah @UnderDogg @nielsdrost7 @zeitschlag