[4.0] Null date corrections for Jgrid and Published buttons by infograf768 · Pull Request #26751 · joomla/joomla-cms

infograf768 · 2019-10-21T09:23:28Z

Pull Request for Issue #26660

Summary of Changes

Null Date is now defined by null and no more by Factory::getDbo()->getNullDate()

Testing Instructions

Create a site or admin module and enter a date for the Start Publishing.
Although no Finish Publishing date is set, the tooltip will display a wrong finish date.

Before patch

After patch

Note:

Remains in core 3 files with instances of Factory::getDbo()->getNullDate()

/layouts/joomla/content/icons/edit.php:25: 		|| ((strtotime($article->publish_down) < strtotime(Factory::getDate())) && $article->publish_down != Factory::getDbo()->getNullDate()))
/layouts/joomla/content/icons/edit.php:35: 		|| ((strtotime($article->publish_down) < strtotime(Factory::getDate())) && $article->publish_down != Factory::getDbo()->getNullDate()))
/libraries/src/Form/Field/CalendarField.php:228: 				if ($this->value && $this->value != Factory::getDbo()->getNullDate())
/libraries/src/Form/Field/CalendarField.php:240: 				if ($this->value && $this->value != Factory::getDbo()->getNullDate())
/libraries/src/Form/Field/CalendarField.php:253: 		if ($this->value && $this->value != Factory::getDbo()->getNullDate() && strtotime($this->value) !== false)
/libraries/src/HTML/HTMLHelper.php:1115: 		if ($value && $value !== Factory::getDbo()->getNullDate() && strtotime($value) !== false)

I did not touch these as I was not sure where to test them.

richard67 · 2019-10-21T09:40:04Z

@infograf768 Was on my task list, but you were faster. Code looks good to me. Will test tonight after work.

infograf768 · 2019-10-21T09:50:30Z

@richard67
There are many other variations of ->getNullDate ;)

richard67 · 2019-10-21T09:59:04Z

@infograf768 I know. Not all of them shall be thrown away, some of them we need to keep for 3rd party extensions. But a check for real NULL should be added. Maybe @wilsonge can check hf the changes here are ok. To me they seem to be ok.

ChristineWk · 2019-10-21T10:06:07Z

I have tested this item ✅ successfully on d5feaae

Confirm Before Patch & After Patch, but on both I hv this.
Info: Tested on beta-1 (nightly from yesterday)

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26751.}

ChristineWk · 2019-10-21T10:09:50Z

see attached.

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26751.}

infograf768 · 2019-10-21T10:16:23Z

@ChristineWk
The date is in the past.
Please try to also modify Factory::getDbo()->getNullDate() to null in

/layouts/joomla/content/icons/edit.php:25: 		|| ((strtotime($article->publish_down) < strtotime(Factory::getDate())) && $article->publish_down != Factory::getDbo()->getNullDate()))
/layouts/joomla/content/icons/edit.php:35: 		|| ((strtotime($article->publish_down) < strtotime(Factory::getDate())) && $article->publish_down != Factory::getDbo()->getNullDate()))

HLeithner · 2019-10-21T10:22:47Z

If we want to support 3rd party extensions with this layout which doesn't null as date type we have to keep getNullDate and just add a === NULL to the if-statement.

If we don't want to support this, you can remove the $nullDate variable completely. Since we still have getNullDate in the near future (4.x) we should support it in the layout too.

Please change
($publish_up != $nullDate)
to
($publish_up !== NULL &&$publish_up != $nullDate)

thx

richard67 · 2019-10-21T10:26:15Z

Please change
($publish_up != $nullDate)
to
($publish_up !== NULL &&$publish_up != $nullDate)

Should be lowercase nullfor PHP CS.

@infograf768 In PublishedButton.php the checks in lines 89 and 95 should also be corrected. They were wrong before nulldate changes, too, because in lines 63 and 64 the datetimes are set to null. If you want I can check tonight after work and send you a PR to your branch.

infograf768 · 2019-10-21T10:28:09Z

@richard67

If you want I can check tonight after work and send you a PR to your branch.

Thanks, please do and correct all.

richard67 · 2019-10-21T10:29:54Z

@wilsonge Shall JGrid or the Published Buttons be usable by 3rd party extensions so we have to keep old nulldate checks here?

ChristineWk · 2019-10-21T10:37:32Z

@infograf768

Trashed previous and made new with tomorrow date :-)

Hv also to read new comments. Should I delete successful?

brianteeman · 2019-10-21T12:30:30Z

@wilsonge Shall JGrid or the Published Buttons be usable by 3rd party extensions so we have to keep old nulldate checks here?

Of course they must be - they are part of the librarries

wilsonge · 2019-10-21T15:10:41Z

Yup we need to keep compat in the libraries please. but we can definitely mark as deprecated

[4.0] Check for both old (pseudo-)null dates and real NULL values in Jgrid and Published buttons

infograf768 · 2019-10-21T16:26:38Z

I merged @richard67 patch into this. It works fine for me.

Do we also need to modify /layouts/joomla/content/icons/edit.php ?
It looks like it is only used for legacy:

$text = LayoutHelper::render('joomla.content.icons.edit', array('article' => $article, 'overlib' => $overlib, 'legacy' => $legacy));

@ChristineWk
Please test again.

richard67 · 2019-10-21T16:38:37Z

@wilsonge I don't know what exactly to mark as deprecated.

brianteeman · 2019-10-21T16:39:23Z

Do we also need to modify /layouts/joomla/content/icons/edit.php ?

That was for the icon on front end editing iirc

ChristineWk · 2019-10-21T16:59:41Z

@infograf768

hv similar the same as before.

richard67 · 2019-10-21T17:04:43Z

@ChristineWk If I understand the functionality right, the above is correct. Pending means publish up is in future. Today is the 21st.

richard67 · 2019-10-21T17:28:32Z

Or do I maybe understand the issue wrong?

richard67 · 2019-10-21T17:58:30Z

I have tested this item ✅ successfully on d0128ab

Before patch: A publish down time is shown in the tool tip even if no publish down time was set.

After patch: Publish down time is only shown if really set.

The icon and tool tip is set according to the times (expired if publish down in past, pending if publish up in future, published if publish up in past and publish down in future or no publish down set).

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26751.}

ChristineWk · 2019-10-21T18:16:17Z

I have tested this item ✅ successfully on d0128ab

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26751.}

Quy · 2019-10-21T18:38:53Z

RTC

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/26751.}

richard67 · 2019-10-21T19:48:42Z

If nobody else wants to do that, I will make another PR for /layouts/joomla/content/icons/edit.php ... but it may take a while, not sure if I will get it finished otday with testing instructions.

infograf768 · 2019-10-25T05:10:25Z

@Quy @wilsonge
Please merge.

wilsonge · 2019-10-25T20:47:21Z

Thanks!

[4.0] Null date corrections for Jgrid and Published buttons

d5feaae

joomla-cms-bot added the PR-4.0-dev label Oct 21, 2019

infograf768 mentioned this pull request Oct 21, 2019

[4.0] Administrator: Misleading tooltip for status in modules list (administrator) #26660

Closed

richard67 added 2 commits October 21, 2019 15:05

Update jgrid.php

1781328

Update PublishedButton.php

3670d83

richard67 mentioned this pull request Oct 21, 2019

[4.0] Check for both old (pseudo-)null dates and real NULL values in Jgrid and Published buttons infograf768/joomla-cms#53

Merged

Merge pull request #53 from richard67/patch-1

d0128ab

[4.0] Check for both old (pseudo-)null dates and real NULL values in Jgrid and Published buttons

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Oct 21, 2019

richard67 mentioned this pull request Oct 21, 2019

[4.0] Null date corrections for frontent edit icon #26756

Merged

infograf768 added 3 commits October 22, 2019 06:55

Merge branch '4.0-dev' into 4.0_nulldate

20b4c96

Merge branch '4.0-dev' into 4.0_nulldate

952d34c

Merge branch '4.0-dev' into 4.0_nulldate

b0dfb49

wilsonge merged commit 6215038 into joomla:4.0-dev Oct 25, 2019

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Oct 25, 2019

wilsonge added this to the Joomla 4.0 milestone Oct 25, 2019

Uh oh!

Conversation

infograf768 commented Oct 21, 2019

Summary of Changes

Testing Instructions

Before patch

After patch

Note:

Uh oh!

richard67 commented Oct 21, 2019

Uh oh!

infograf768 commented Oct 21, 2019

Uh oh!

richard67 commented Oct 21, 2019

Uh oh!

ChristineWk commented Oct 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ChristineWk commented Oct 21, 2019

Uh oh!

infograf768 commented Oct 21, 2019

Uh oh!

HLeithner commented Oct 21, 2019

Uh oh!

richard67 commented Oct 21, 2019

Uh oh!

infograf768 commented Oct 21, 2019

Uh oh!

richard67 commented Oct 21, 2019

Uh oh!

ChristineWk commented Oct 21, 2019

Uh oh!

brianteeman commented Oct 21, 2019

Uh oh!

wilsonge commented Oct 21, 2019

Uh oh!

infograf768 commented Oct 21, 2019

Uh oh!

richard67 commented Oct 21, 2019

Uh oh!

brianteeman commented Oct 21, 2019

Uh oh!

ChristineWk commented Oct 21, 2019

Uh oh!

richard67 commented Oct 21, 2019

Uh oh!

richard67 commented Oct 21, 2019

Uh oh!

richard67 commented Oct 21, 2019

Uh oh!

ChristineWk commented Oct 21, 2019

Uh oh!

Quy commented Oct 21, 2019

Uh oh!

richard67 commented Oct 21, 2019

Uh oh!

infograf768 commented Oct 25, 2019

Uh oh!

wilsonge commented Oct 25, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ChristineWk commented Oct 21, 2019 •

edited

Loading