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

Add logic for reminder generation #219

Merged
merged 31 commits into from
Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
29e947e
first commit
Mar 11, 2024
5ed3972
make class parameers optional / change classnames
Mar 11, 2024
f745895
add ability to distinguish between attachments
Mar 20, 2024
4489eb6
repair shemadb_default
Mar 20, 2024
000e1aa
remove superflous line
Mar 20, 2024
adccafb
remove lacalization
Mar 20, 2024
e76d759
add "create_reminder" to localization
Mar 20, 2024
2d2e6ad
remove lacalization from admin folder
Mar 20, 2024
ef788ca
repaired midgard l10n
Mar 20, 2024
1bcc255
make class name dynamic
Mar 21, 2024
e12885a
add typehint to get_attachment
Mar 21, 2024
7a6b8ed
hide button, when not needed / codingstyle
Mar 21, 2024
6374582
reminder button appers after 14 days
Mar 22, 2024
e2c5b00
use overdue for status
Mar 22, 2024
d49b802
remove $invoice variable /change reminder classname
Mar 25, 2024
6b24e4c
change seccess / error message reminder pdf creation
Mar 25, 2024
c059462
change config /remove lacalization
Mar 25, 2024
9072a3a
change dialog window on reminder re-creation
Mar 25, 2024
ff1df3e
remove 'kind switch'
Mar 25, 2024
7b987a9
remove "has alredy sent" message for overdue reminder
Mar 26, 2024
f0c7a80
include reminder view
Mar 26, 2024
58753de
remove nested if from populate_toolbar()
Mar 26, 2024
0329955
add l10n to filename
Mar 27, 2024
3553315
coding style / remove $kind variable / change l10n
Apr 2, 2024
4179a8c
change label name to reminder
Apr 2, 2024
1d20f78
remove bracelets
Apr 2, 2024
a2445c6
add reminder to l10n
Apr 3, 2024
630d85a
change Mahnung to mahnung.
Apr 4, 2024
764d7a9
change: title/spelling/logic
Apr 8, 2024
428c193
change mahnung lcfirst
Apr 8, 2024
e7dab81
add test
Apr 9, 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
2 changes: 1 addition & 1 deletion lib/midcom/admin/folder/locale/default.de.txt
Original file line number Diff line number Diff line change
Expand Up @@ -220,4 +220,4 @@ Stilvorlage

---STRING objects with no navigation entry are still accessible on-site if the url is known
Objekte ohne Navigationseintrag sind weiterhin verfügbar, insofern die URL bekannt ist
---STRINGEND
---STRINGEND
Copy link
Owner

Choose a reason for hiding this comment

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

why is this in the PR?

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 didn't see any changes. perhaps my IDE removed a space on saving.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gets removed with the next comit

Copy link
Owner

Choose a reason for hiding this comment

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

it's still here..

2 changes: 1 addition & 1 deletion lib/midcom/admin/folder/locale/default.en.txt
Original file line number Diff line number Diff line change
Expand Up @@ -220,4 +220,4 @@ Style template

---STRING objects with no navigation entry are still accessible on-site if the url is known
Objects that are hidden from navigation are still accessible on-site if the URL is known
---STRINGEND
---STRINGEND
1 change: 1 addition & 0 deletions lib/org/openpsa/invoices/config/config.inc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
'invoice_number_format' => '#%06d',
//contains the class file to create the pdf for the invoice, if set to false pdf-creation is disabled
'invoice_pdfbuilder_class' => false, //implements org_openpsa_invoices_interfaces_pdfbuilder
'invoice_pdfbuilder_reminder_class' => false,
Copy link

Choose a reason for hiding this comment

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

Consider enhancing the comment for the invoice_pdfbuilder_reminder_class configuration option to provide more context. For example, explaining that setting this option to a class name enables PDF creation for invoice reminders and that the class should implement a specific interface or follow a certain pattern for compatibility. This additional detail can help developers understand the purpose and usage of this configuration option more clearly.

-//contains the class file to create the pdf for the invoice, if set to false pdf-creation is disabled
+// Specifies the class file to create PDFs for invoice reminders. If set to false, PDF creation for reminders is disabled. 
+// When enabled, the specified class should implement the org_openpsa_invoices_interfaces_pdfbuilder interface or follow a compatible pattern.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
'invoice_pdfbuilder_reminder_class' => false,
// Specifies the class file to create PDFs for invoice reminders. If set to false, PDF creation for reminders is disabled.
// When enabled, the specified class should implement the org_openpsa_invoices_interfaces_pdfbuilder interface or follow a compatible pattern.
'invoice_pdfbuilder_reminder_class' => false,


'invoice_mail_bcc' => false,
'invoice_mail_from_address' => '[email protected]',
Expand Down
4 changes: 4 additions & 0 deletions lib/org/openpsa/invoices/config/routes.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ create_pdf:
path: /invoice/action/create_pdf/
defaults: { _controller: 'org_openpsa_invoices_handler_invoice_action::create_pdf' }

create_reminder:
path: /invoice/action/create_reminder/
defaults: { _controller: 'org_openpsa_invoices_handler_invoice_action::create_pdf_reminder' }

recalc_invoice:
path: /invoice/recalculation/{guid}/
defaults: { _controller: 'org_openpsa_invoices_handler_invoice_items::recalculation' }
Expand Down
11 changes: 11 additions & 0 deletions lib/org/openpsa/invoices/config/schemadb_default.inc
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,17 @@
'index_method' => 'attachment',
'hidden' => true,
],
'pdf_file_reminder' => [
'title' => 'pdf file',
SimonRautenberg marked this conversation as resolved.
Show resolved Hide resolved
'type' => 'blobs',
'widget' => 'downloads',
'type_config' => [
'sortable' => false,
'max_count' => 1,
],
'index_method' => 'attachment',
'hidden' => true,
],
'files' => [
'title' => 'Files',
'type' => 'blobs',
Expand Down
11 changes: 11 additions & 0 deletions lib/org/openpsa/invoices/handler/invoice/action.php
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,17 @@ public function _handler_create_pdf()
}
}

public function _handler_create_pdf_reminder()
{
$pdf_helper = new org_openpsa_invoices_invoice_pdf($this->invoice);
try {
$pdf_helper->render_and_attach('reminder');
return $this->reply(true, $this->_l10n->get('reminder pdf created'));
} catch (midcom_error $e) {
return $this->reply(false, $this->_l10n->get('reminder pdf creation failed') . ': ' . $e->getMessage());
}
}

public function _handler_send_by_mail()
{
$customerCard = org_openpsa_widgets_contact::get($this->invoice->customerContact);
Expand Down
9 changes: 8 additions & 1 deletion lib/org/openpsa/invoices/handler/invoice/view.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public function _handler_read(string $guid, array &$data)
$data['object_view'] = $dm->get_content_html();
$data['invoice_items'] = $this->invoice->get_invoice_items();

$this->populate_toolbar();
$this->populate_toolbar($this->invoice);
SimonRautenberg marked this conversation as resolved.
Show resolved Hide resolved
$this->update_breadcrumb();

midcom::get()->metadata->set_request_metadata($this->invoice->metadata->revised, $guid);
Expand Down Expand Up @@ -98,6 +98,13 @@ private function populate_toolbar()
}
}

if ($this->_config->get('invoice_pdfbuilder_reminder_class') && $this->invoice->get_status() == 'overdue') {
$button = $this->build_button('create_reminder', 'file-pdf-o');
$pdf_helper = new org_openpsa_invoices_invoice_pdf($this->invoice);
$button[MIDCOM_TOOLBAR_OPTIONS] = $pdf_helper->get_button_options('reminder');
$buttons[] = $button;
SimonRautenberg marked this conversation as resolved.
Show resolved Hide resolved
}

if ($this->invoice->is_cancelable()) {
$buttons[] = $this->build_button('create_cancelation', 'ban');
}
Expand Down
39 changes: 27 additions & 12 deletions lib/org/openpsa/invoices/invoice/pdf.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,13 @@ public function __construct(org_openpsa_invoices_invoice_dba $invoice)
$this->invoice = $invoice;
}

public function get_attachment(bool $autocreate = false) : ?midcom_db_attachment
public function get_attachment(bool $autocreate = false, string $kind = null) : ?midcom_db_attachment
{
$pdf_files = blobs::get_attachments($this->invoice, "pdf_file");
if ($kind == 'reminder') {
$pdf_files = blobs::get_attachments($this->invoice, "pdf_file_reminder");
} else {
$pdf_files = blobs::get_attachments($this->invoice, "pdf_file");
}
if (!empty($pdf_files)) {
return reset($pdf_files);
}
Expand All @@ -33,17 +37,17 @@ public function get_attachment(bool $autocreate = false) : ?midcom_db_attachment
return $this->render_and_attach();
}

public function get_button_options() : array
public function get_button_options(string $kind = 'invoice') : array
{
if ($attachment = $this->get_attachment()) {
if ($this->invoice->sent) {
$message = 'invoice has already been sent. should it be replaced?';
if ($this->invoice->sent && (!($kind == 'reminder' && $this->invoice->get_status() == 'overdue'))) {
SimonRautenberg marked this conversation as resolved.
Show resolved Hide resolved
$message = $kind . ' has already been sent. should it be replaced?';
SimonRautenberg marked this conversation as resolved.
Show resolved Hide resolved
}
// check if auto generated parameter is same as md5 in current-file
// if not the file was manually uploaded
elseif ($checksum = $attachment->get_parameter('org.openpsa.invoices', 'auto_generated')) {
if ($checksum !== md5_file($attachment->get_path())) {
$message = 'current pdf file was manually uploaded shall it be replaced ?';
$message = 'current pdf file was manually uploaded shall it be replaced ?';
SimonRautenberg marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand All @@ -63,9 +67,14 @@ public function get_button_options() : array
];
}

public function render_and_attach() : midcom_db_attachment
public function render_and_attach(string $kind = null) : midcom_db_attachment
SimonRautenberg marked this conversation as resolved.
Show resolved Hide resolved
{
$client_class = midcom_baseclasses_components_configuration::get('org.openpsa.invoices', 'config')->get('invoice_pdfbuilder_class');
if ($kind == null) {
$client_class = midcom_baseclasses_components_configuration::get('org.openpsa.invoices', 'config')->get('invoice_pdfbuilder_class');
} else {
$client_class = midcom_baseclasses_components_configuration::get('org.openpsa.invoices', 'config')->get('invoice_pdfbuilder_' . $kind . '_class');
}

SimonRautenberg marked this conversation as resolved.
Show resolved Hide resolved
if (!class_exists($client_class)) {
throw new midcom_error('Could not find PDF renderer ' . $client_class);
}
Expand All @@ -78,15 +87,21 @@ public function render_and_attach() : midcom_db_attachment
}
// renders the pdf and attaches it to the invoice
$pdf_builder = new $client_class($this->invoice);
$filename = midcom_helper_misc::urlize($this->invoice->get_label()) . '.pdf';
$l10n = midcom::get()->i18n->get_l10n('org.openpsa.invoices');
if ($kind == 'reminder') {
$name = "guids_pdf_file_reminder";
$filename = midcom_helper_misc::urlize($this->invoice->get_label()) . '_' . $l10n->get('reminder') . '.pdf';
} else {
$name = "guids_pdf_file";
$filename = midcom_helper_misc::urlize($this->invoice->get_label()) . '.pdf';
}

// tmp filename
$tmp_file = midcom::get()->config->get('midcom_tempdir') . "/" . $filename;

// render pdf to tmp filename
$pdf_builder->render($tmp_file);

$attachment = $this->get_attachment();
$attachment = $this->get_attachment(false, $kind);
if ($attachment) {
$attachment->name = $filename;
$attachment->title = $this->invoice->get_label();
Expand All @@ -95,7 +110,7 @@ public function render_and_attach() : midcom_db_attachment
} else {
$attachment = $this->invoice->create_attachment($filename, $this->invoice->get_label(), "application/pdf");
if ( !$attachment
|| !$this->invoice->set_parameter("midcom.helper.datamanager2.type.blobs", "guids_pdf_file", $attachment->guid . ":" . $attachment->guid)) {
|| !$this->invoice->set_parameter("midcom.helper.datamanager2.type.blobs", $name, $attachment->guid . ":" . $attachment->guid)) {
throw new midcom_error("Failed to create invoice attachment for pdf: " . midcom_connection::get_error_string());
}
}
Expand Down
16 changes: 16 additions & 0 deletions lib/org/openpsa/invoices/locale/default.de.txt
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ Rechnungsdaten
Diese Rechnung wurde bereits verschickt. Soll die Datei trotzdem neu erzeugt werden?
---STRINGEND

---STRING reminder has already been sent. should it be replaced?
Diese Mahnung wurde bereits verschickt. Soll die Datei trotzdem neu erzeugt werden?
---STRINGEND
Copy link

Choose a reason for hiding this comment

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

The addition of the string for handling cases where a reminder has already been sent is clear and correctly translates the intended message. However, to maintain consistency with the rest of the file, consider starting the sentence with an uppercase letter.

- ---STRING reminder has already been sent. should it be replaced?
+ ---STRING Reminder has already been sent. Should it be replaced?

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
---STRING reminder has already been sent. should it be replaced?
Diese Mahnung wurde bereits verschickt. Soll die Datei trotzdem neu erzeugt werden?
---STRINGEND
---STRING Reminder has already been sent. Should it be replaced?
Diese Mahnung wurde bereits verschickt. Soll die Datei trotzdem neu erzeugt werden?
---STRINGEND


---STRING invoice items
Rechnungspositionen
---STRINGEND
Expand Down Expand Up @@ -414,14 +418,26 @@ Die derzeitige PDF-Datei wurde manuell hochgeladen. Soll diese Datei wirklich er
PDF erzeugen
---STRINGEND

---STRING create_reminder
Mahnung erzeugen
---STRINGEND

---STRING pdf created
PDF-Datei wurde erfolgreich erzeugt
---STRINGEND

---STRING reminder pdf created
Mahnungs PDF-Datei wurde erfolgreich erzeugt
---STRINGEND
Comment on lines +429 to +431
Copy link

Choose a reason for hiding this comment

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

Ensure the translation for "reminder pdf created" is consistent with the updated terminology.

- Mahnungs PDF-Datei wurde erfolgreich erzeugt
+ Zahlungserinnerungs-PDF wurde erfolgreich erstellt

This adjustment aligns the terminology with the suggested changes for "reminder" and "create_reminder", maintaining consistency across the application.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
---STRING reminder pdf created
Mahnungs PDF-Datei wurde erfolgreich erzeugt
---STRINGEND
---STRING reminder pdf created
Zahlungserinnerungs-PDF wurde erfolgreich erstellt
---STRINGEND


---STRING pdf creation failed
PDF-Datei konnte nicht erzeugt werden
---STRINGEND

---STRING reminder pdf creation failed
Mahnungs PDF-Datei konnte nicht erzeugt werden
---STRINGEND
Comment on lines +437 to +439
Copy link

Choose a reason for hiding this comment

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

Revise the translation for "reminder pdf creation failed" to match the updated terminology.

- Mahnungs PDF-Datei konnte nicht erzeugt werden
+ Erstellung der Zahlungserinnerungs-PDF fehlgeschlagen

This modification ensures consistency in terminology and clarifies the error message for users.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
---STRING reminder pdf creation failed
Mahnungs PDF-Datei konnte nicht erzeugt werden
---STRINGEND
---STRING reminder pdf creation failed
Erstellung der Zahlungserinnerungs-PDF fehlgeschlagen
---STRINGEND


---STRING position
Position
---STRINGEND
8 changes: 8 additions & 0 deletions lib/org/openpsa/invoices/locale/default.en.txt
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ Invoice data
This invoice has already been sent. Should the file be regenerated anyways?
---STRINGEND

---STRING reminder has already been sent. should it be replaced?
This reminder has already been sent. Should the file be regenerated anyways?
---STRINGEND
SimonRautenberg marked this conversation as resolved.
Show resolved Hide resolved

---STRING invoice items
Invoice items
---STRINGEND
Copy link

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [413-414]

The punctuation and spacing around question marks should be consistent with standard English grammar rules. Consider the following adjustments for clarity and consistency:

- shall it be replaced ? The current PDF file was manually uploaded. So shall it really be replaced ?
+ shall it be replaced? The current PDF file was manually uploaded. So, shall it really be replaced?

Expand Down Expand Up @@ -425,3 +429,7 @@ Could not create PDF
---STRING position
Position
---STRINGEND

---STRING create_reminder
create reminder
---STRINGEND
Comment on lines +429 to +431
Copy link

Choose a reason for hiding this comment

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

The string value for "create reminder" should follow the capitalization style of other UI elements for consistency. Consider capitalizing the first letter of each word:

---STRING create_reminder
-create reminder
+Create Reminder

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
---STRING create_reminder
create reminder
---STRINGEND
---STRING create_reminder
Create Reminder
---STRINGEND

4 changes: 4 additions & 0 deletions lib/org/openpsa/invoices/style/admin-read.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@
<p><strong><?php echo $data['l10n']->get('pdf file'); ?></strong></p>
<?php echo org_openpsa_helpers_fileinfo::render($invoice, 'pdf_file');
}
if ($view['pdf_file_reminder'] != "") { ?>
<p><strong><?php echo $data['l10n']->get('pdf file'); ?></strong></p>
SimonRautenberg marked this conversation as resolved.
Show resolved Hide resolved
<?php echo org_openpsa_helpers_fileinfo::render($invoice, 'pdf_file_reminder');
}
Copy link

Choose a reason for hiding this comment

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

The conditional block for displaying the PDF file reminder is correctly implemented and integrates well with the existing code. However, the label used for the reminder PDF is the same as the one used for the original invoice PDF, which could lead to confusion for users. Consider using a distinct label for the reminder PDF to clearly differentiate it from the original invoice PDF.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if ($view['pdf_file_reminder'] != "") { ?>
<p><strong><?php echo $data['l10n']->get('pdf file'); ?></strong></p>
<?php echo org_openpsa_helpers_fileinfo::render($invoice, 'pdf_file_reminder');
}
if ($view['pdf_file_reminder'] != "") { ?>
<p><strong><?php echo $data['l10n']->get('pdf file'); ?></strong></p>
<?php echo org_openpsa_helpers_fileinfo::render($invoice, 'pdf_file_reminder');
}


$tabs = [];
if ($expenses_url) {
Expand Down