Problem/Motivation

Split plugins out from #3112298: Replace REQUEST_TIME in classes with direct container access to reduce the size of the changed.

This issue deals specifically with replace those calls in plugins with direct container access.

The plugins and classes with direct container access that were identified to have those calls are:

  • \Drupal\datetime\Plugin\views\argument\Date
  • \Drupal\history\Plugin\views\filter\HistoryUserTimestamp
  • \Drupal\views\Plugin\views\argument\Date
  • \Drupal\views\Plugin\views\cache\Time
  • \Drupal\views\Plugin\views\field\Date
  • \Drupal\Core\Field\Plugin\Field\FieldFormatter\TimestampFormatter

Proposed resolution

- Remove deprecated uses of REQUEST_TIME and time() and others.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3395986

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

quietone created an issue. See original summary.

quietone opened merge request !5095

quietone’s picture

quietone commented
Assigned: quietone » Unassigned

Spokje made their first commit to this issue’s fork.

spokje’s picture

spokje
Primary language Dutch
commented
Status: Active » Needs review

Seems like the excellent work of @quietone was overlooked due to this issue being stuck on Active instead of NR.

Updated the deprecation notices and CR to mention 10.3.0 and removed the now deemed superfluous deprecation tests.

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented

Added a few suggestions so this one is consistent with #3113971: Replace REQUEST_TIME in services, see the commit https://git.drupalcode.org/project/drupal/-/commit/fd75fa213472e2029fded...

Then, I see other issue here: #3112295: Replace REQUEST_TIME in rest of OO code (except for tests), but it seems that at least some of the examples from that issue actually should be here like field plugins?

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Status: Needs review » Needs work

andypost made their first commit to this issue’s fork.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

Addressed feedback (there was some typos)

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Left a comment.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review

Fixed CR link

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Verified CR link is fixed and appears rest of threads have been addressed

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Assigned: Unassigned » taran2l
Status: Reviewed & tested by the community » Needs work

There is at least one more place where plugin with direct access exists, setting back to needs work ..

The identified plugins:
- \Drupal\Core\Field\Plugin\Field\FieldFormatter\TimestampFormatter

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Issue summary: View changes
Status: Needs work » Needs review

I've manually checked phpstan baseline file and all other occurrences are either in OO code without container access, or in tests (there are separate issues for that)

Setting back to NR

PS Added a tiny change to UpdateProcessor which slipped away from the #3113971: Replace REQUEST_TIME in services. I think this is a better place than other open issues

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Thanks for CR update, rtbc++

taran2l’s picture

taran2l
Primary language Ukrainian
Location Lviv
commented
Assigned: taran2l » Unassigned

OK, should be good to go now

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Went to commit this but it needs a rebase for phpstan-baseline.neon

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Reviewed & tested by the community

Rebase only (with a PHPStan baseline run), so back to RTBC

  • catch committed 86aacdff on 11.x 
    Issue #3395986 by Taran2L, quietone, Spokje, andypost, smustgrave:...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

quietone closed merge request !5095