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

WORKDAY function returns a wrong date when weekday is Thursday or Friday #1936

Closed
robertotremonti opened this issue Mar 18, 2021 · 2 comments
Closed

WORKDAY function returns a wrong date when weekday is Thursday or Friday #1936

robertotremonti opened this issue Mar 18, 2021 · 2 comments

Comments

@robertotremonti
Copy link

Steps to reproduce

<?php

require __DIR__ . '/vendor/autoload.php';

$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();
$worksheet = $spreadsheet->getActiveSheet();

// monday
$worksheet->setCellValue("A1", "2021-03-15");
$worksheet->setCellValue("A2", "=TEXT(WORKDAY(A1, 3), \"yyyy-mm-dd\")");
// tuesday
$worksheet->setCellValue("A3", "2021-03-16");
$worksheet->setCellValue("A4", "=TEXT(WORKDAY(A3, 3), \"yyyy-mm-dd\")");
// wednesday
$worksheet->setCellValue("A5", "2021-03-17");
$worksheet->setCellValue("A6", "=TEXT(WORKDAY(A5, 3), \"yyyy-mm-dd\")");
// thrusday
$worksheet->setCellValue("A7", "2021-03-18");
$worksheet->setCellValue("A8", "=TEXT(WORKDAY(A7, 3), \"yyyy-mm-dd\")");
// friday
$worksheet->setCellValue("A9", "2021-03-19");
$worksheet->setCellValue("A10", "=TEXT(WORKDAY(A9, 3), \"yyyy-mm-dd\")");
// saturday
$worksheet->setCellValue("A11", "2021-03-20");
$worksheet->setCellValue("A12", "=TEXT(WORKDAY(A11, 3), \"yyyy-mm-dd\")");
// sunday
$worksheet->setCellValue("A13", "2021-03-21");
$worksheet->setCellValue("A14", "=TEXT(WORKDAY(A13, 3), \"yyyy-mm-dd\")");

echo "<pre>";
// 2021-03-18: correct
echo $worksheet->getCell("A2")->getCalculatedValue() . PHP_EOL;
// 2021-03-19: correct
echo $worksheet->getCell("A4")->getCalculatedValue() . PHP_EOL;
// 2021-03-22: correct
echo $worksheet->getCell("A6")->getCalculatedValue() . PHP_EOL;
// 2021-03-22: wrong, expected 2021-03-23
echo $worksheet->getCell("A8")->getCalculatedValue() . PHP_EOL;
// 2021-03-22: wrong, expected 2021-03-24
echo $worksheet->getCell("A10")->getCalculatedValue() . PHP_EOL;
// 2021-03-24: correct
echo $worksheet->getCell("A12")->getCalculatedValue() . PHP_EOL;
// 2021-03-24: correct
echo $worksheet->getCell("A14")->getCalculatedValue() . PHP_EOL;
echo "</pre>";
?>

PhpSpreadsheet and PHP versions

PhpSpreadsheet 1.17.1
PHP 7.4.12 and PHP 8.0.0
@oleibman oleibman mentioned this issue Mar 19, 2021
Merged
5 tasks
MarkBaker pushed a commit that referenced this issue Mar 21, 2021
@oleibman
Complete Breakup Of Calculation/DateTime Functions (#1937)
9beacd2 
* Complete Breakup Of Calculation/DateTime Functions

In conjunction with parallel breakups happening in other areas of Calculation, this change breaks up all the DateTime functions into their own classes. All methods remaining in DateTime itself have a doc block deprecation notice, and consist only of stub code to call the replacement methods. Coverage of DateTime itself and all the replacement methods is 100%.

There is only one substantive change to the code (see next paragraph). Among the non-substantive changes, it now adopts the same parsing technique (throwing and catching exceptions) already in use in Engineering and MathTrig. Boolean parameters are allowed in lieu of numbers when Excel allows them. Most of the code changes involve refactoring due to the need to avoid Scrutinizer "complexity" failures in what it will consider to be new methods.

Issue #1936 was opened just as I was staging this. It is now fixed. One existing WORKDAY test was wrong (noted in a comment in the test data file), and a bunch of new tests are added.

I found it confusing to use DateTime as a node of the the class name since most of the methods invoke native DateTime methods. So, everything is moved to directory DateTimeExcel, and that is what is used in the class names.

There are several follow-up activities that I am planning to undertake if this PR is merged.

- ODS supports dates well before 1900. There are exactly 2 assertions for this functionality. More are needed (and some functions might have to change to accept this).
- WEEKDAY has some poorly documented extra options for "style" which are not yet implemented.
- Most tests have been changed to use a formula as entered on a spreadsheet rather than a direct call to the method which implements the formula. There are 3 exceptions at this time. WORKDAY and NETWORKDAYS, which include arrays as part of their parameters, are more complicated than most. YEARFRAC was just too large to deal with now.
- There are direct calls to the now-deprecated methods in both source code and tests, mostly in Financial code, but possibly in others as well. These need to be changed.
- Some constants, none "officially" documented, remain in the original class. These should be either deleted or marked deprecated. I wasn't sure if deprecation was even possible (or desirable), and did not want that to be something which would cause Scrutinizer to fail the change.

* Deprecate Now-unused Constants, Fix Yearfrac bug, Change 3 Tests

Add new DateTime/Constants class, initially populated with constants used in Weeknum.

MS has another inconsistency with how it handles null cells in Yearfrac. Change PhpSpreadsheet to behave compatibly with this bug.

I have modified YearFrac, WorkDay, and NetworkDays tests to be more to my liking. Many tests added to YearFrac because of the bug above. Only minor modifications to the existing tests for the others.
@oleibman
Copy link
Collaborator

oleibman commented Apr 8, 2021

Does the push mentioned above solve the problem, and is it okay to close the ticket now?

@robertotremonti
Copy link
Author

@oleibman My code above works fine with your latest push. You can close the ticket. Thanks.

@oleibman oleibman closed this as completed Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@robertotremonti @oleibman