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: Invoice subtypes for customers and vendors #26233

Merged
merged 9 commits into from
Oct 18, 2023
Merged

NEW: Invoice subtypes for customers and vendors #26233

merged 9 commits into from
Oct 18, 2023

Conversation

sonikf
Copy link
Contributor

@sonikf sonikf commented Oct 17, 2023

Re up

if ($table === 'facture' || $table === 'facture_fourn') {
$sql = "SELECT s.label FROM " . MAIN_DB_PREFIX . $table . " AS f";
$sql .= " INNER JOIN " . MAIN_DB_PREFIX . "c_invoice_subtype AS s ON f.subtype = s.rowid";
$sql .= " WHERE f.ref = \"$this->ref\"";
Copy link
Member

@eldy eldy Oct 17, 2023

Choose a reason for hiding this comment

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

Possible SQL Injection here.
Use
$sql .= " WHERE f.ref = '".$this->db->escape($this->ref)."'";
The quotes for SQL must be ' and content must be escaped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

break;
}
}
if (in_array($selectedcode, array('5.1', '5.2', '11.4'))) {
Copy link
Member

@eldy eldy Oct 17, 2023

Choose a reason for hiding this comment

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

Hard coded value are not allowed. I suggest to remove completely this check for the first version of 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.

Fixed
Any suggestion on how i could tackle this because it's about the most common mistake made by users having to choose between so many subtypes and is driving accountants crazy!?

fyi the hardcoded values i used are from the revenue ministry api and never gonna change.

break;
}
}
if (in_array($selectedcode, array('5.1', '5.2', '11.4'))) {
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@eldy eldy added the PR to fix - OK to merge if suggested fix are done PR was analyzed by PR merger and seems ok to be merged as soon as a fix has been published label Oct 17, 2023
@sonikf
Copy link
Contributor Author

sonikf commented Oct 17, 2023

@eldy
Testing my changes using develop i got
Fatal error: Uncaught Error: Class 'Facture' not found in /fourn/facture/list.php:1524 Stack trace: #0 {main} thrown in /fourn/facture/list.php on line 1524

following code gives the error and it is actually used in /compta/facture/list.php with out errors.
I tried changing Facture to FactureFournisseur as i thought to be a typo but then i got
Fatal error: Uncaught Error: Call to a member function getAvailableDiscounts() on null
Any idea?


	if ($facturestatic->status == Facture::STATUS_CLOSED && $facturestatic->close_code == 'discount_vat') {		// If invoice closed with discount for anticipated payment
		$remaintopay = 0;
		$multicurrency_remaintopay = 0;
	}
	if ($facturestatic->type == Facture::TYPE_CREDIT_NOTE && $obj->paye == 1) {		// If credit note closed, we take into account the amount not yet consumed
		$remaincreditnote = $discount->getAvailableDiscounts($companystatic, '', 'rc.fk_facture_source='.$facturestatic->id);
		$remaintopay = -$remaincreditnote;
		$totalpay = price2num($facturestatic->total_ttc - $remaintopay);
		$multicurrency_remaincreditnote = $discount->getAvailableDiscounts($companystatic, '', 'rc.fk_facture_source='.$facturestatic->id, 0, 0, 1);
		$multicurrency_remaintopay = -$multicurrency_remaincreditnote;
		$multicurrency_totalpay = price2num($facturestatic->multicurrency_total_ttc - $multicurrency_remaintopay);
	}

@sonikf sonikf requested a review from eldy October 17, 2023 16:33
@eldy eldy merged commit 732fb2f into Dolibarr:develop Oct 18, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR to fix - OK to merge if suggested fix are done PR was analyzed by PR merger and seems ok to be merged as soon as a fix has been published
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants