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 Enable auto update PDF on update note #9018

Merged
merged 7 commits into from Sep 2, 2018
Merged

Conversation

all3kcis
Copy link
Contributor

Generate order/invoice PDF on update public note

Generate order/invoice PDF on update public note
@@ -2511,6 +2511,27 @@ function update_note($note,$suffix='')
$this->note = $note; // deprecated
$this->note_private = $note;
}
if($suffix != '_private' && in_array($this->table_element, array('commande_fournisseur', 'commande', 'propal', 'facture_fourn', 'facture' ))){
Copy link
Member

Choose a reason for hiding this comment

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

The "update_note" method is a CRUD method to affect the database values. I would prefer to see code to build PDF that is not related with DAO principle after the call of update_note

@eldy eldy added the PR to fix or conflict to solve PR needs to be fixed to be integrated (except for conflicts, a comment describes the fix to do) label Jun 29, 2018
@all3kcis
Copy link
Contributor Author

all3kcis commented Jul 3, 2018

I can transfer code into "htdocs/core/actions_setnotes.inc.php", Ok ?

@eldy
Copy link
Member

eldy commented Jul 16, 2018

Yes, good idea.

$hidedetails = (GETPOST('hidedetails','int') ? GETPOST('hidedetails','int') : (! empty($conf->global->MAIN_GENERATE_DOCUMENTS_HIDE_DETAILS) ? 1 : 0));
$hidedesc = (GETPOST('hidedesc','int') ? GETPOST('hidedesc','int') : (! empty($conf->global->MAIN_GENERATE_DOCUMENTS_HIDE_DESC) ? 1 : 0));
$hideref = (GETPOST('hideref','int') ? GETPOST('hideref','int') : (! empty($conf->global->MAIN_GENERATE_DOCUMENTS_HIDE_REF) ? 1 : 0));
require_once DOL_DOCUMENT_ROOT.'/fourn/class/fournisseur.product.class.php';
Copy link
Member

Choose a reason for hiding this comment

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

Is this require_once necessary ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why.
There is no dedicated code to supplier product added by the PR, so why adding this ? The require is surely at the wrong place.

@eldy eldy added the PR OK to merge PR was analyzed by PR merger and seems ok to be validated. Merge may occurs soon... label Aug 13, 2018
@eldy eldy merged commit 5488bfe into Dolibarr:develop Sep 2, 2018
@eldy eldy changed the title Enable auto update PDF on update note NEW Enable auto update PDF on update note Sep 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR OK to merge PR was analyzed by PR merger and seems ok to be validated. Merge may occurs soon... PR to fix or conflict to solve PR needs to be fixed to be integrated (except for conflicts, a comment describes the fix to do)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants