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 Add option to change all service dates at once #29348

Merged

Conversation

SylvainLegrand
Copy link
Contributor

Add an icon to the description title
Description-1

This icon open a form
Description-2

Before
Before

After
After

@SylvainLegrand
Copy link
Contributor Author

@eldy

The run Phan analysis shows an error which comes from the original Dolibarr code.
Can we correct this?

@eldy
Copy link
Member

eldy commented Apr 23, 2024

@eldy

The run Phan analysis shows an error which comes from the original Dolibarr code.
Can we correct this?

Yes, you must pull the last version of branch and solve conflict to have such errors fixed

@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 Apr 23, 2024
@eldy eldy changed the title New option to change all service dates at once NEW Add option to change all service dates at once May 10, 2024
@@ -56,7 +56,24 @@
}

// Description
print '<th class="linecoldescription">'.$langs->trans('Description').'</th>';
print '<th class="linecoldescription">'.$langs->trans('Description');
// @phan-suppress-next-line PhanUndeclaredConstantOfClass
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove this comment line ? May hide a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The huge problem is that if I remove this line, the CI/phan check fails.
The second point is that I copy this solution from line 91!

????? What should we do ????? ;)

Copy link
Member

@eldy eldy May 13, 2024

Choose a reason for hiding this comment

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

Remove the line. If CI/phan check fails after, it means we found a bug to fix. So remove it (i can't do it myself, it seems your repo is not open to external contribution) so we can see what is the error message to have a clue of the bug...

@@ -56,7 +56,23 @@
}

// Description
print '<th class="linecoldescription">'.$langs->trans('Description').'</th>';
print '<th class="linecoldescription">'.$langs->trans('Description');
if (in_array($object->element, array('propal', 'commande', 'facture', 'order_supplier', 'invoice_supplier')) && $object->status == $object::STATUS_DRAFT) {
Copy link
Member

@eldy eldy May 13, 2024

Choose a reason for hiding this comment

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

The problem is due to the line
@phan-var-force CommonObject
few line before. It says to phan than $object is CommonObject but it is not, it is Facture or Propal or Commande...

Can you try this workaround:

$constant = get_class($object)."::STATUS_DRAFT";
if (in_array($object->element, array('propal', 'commande', 'facture', 'order_supplier', 'invoice_supplier')) && defined($constant) && $object->status == constant($constant)) {

@eldy
Copy link
Member

eldy commented May 13, 2024

How do you make your push request ? Did you make your commit on an IDE and the create the PR from github or do you edit code from github web ?

@SylvainLegrand
Copy link
Contributor Author

@eldy

The last change you asked me to make I did by editing the file from the github web.

@SylvainLegrand
Copy link
Contributor Author

@eldy

Now it's ready to merge

@eldy eldy merged commit ace675d into Dolibarr:develop May 25, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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