Problem/Motivation

There is a todo in Drupal\user\TempStore to make the expire time configurable. The only way to do this currently is to extend the class and replace it in the container.

Proposed resolution

Use a container parameter instead?

Remaining tasks

User interface changes

None

API changes

None

CommentFileSizeAuthor
#5 2273385-5.patch4.09 KBdamiankloip
#4 interdiff-2273385-4.txt1.45 KBdamiankloip
#4 2273385-4.patch8.26 KBdamiankloip
#2 interdiff-2273385-2.txt768 bytesdamiankloip
#2 2273385-2.patch4.08 KBdamiankloip
user-tempstore-expire-parameter.patch3.33 KBdamiankloip

Comments

Status: Needs review » Needs work

The last submitted patch, user-tempstore-expire-parameter.patch, failed testing.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new 2273385-2.patch4.08 KB
new interdiff-2273385-2.txt768 bytes
dawehner’s picture

dawehner
Primary language German
commented

This patch is awesome! We could really use container parameters all over the place!!

  1. +++ b/core/modules/user/lib/Drupal/user/TempStore.php
@@ -83,10 +80,11 @@ class TempStore {
    * @param mixed $owner
    *   The owner key to store along with the data (e.g. a user or session ID).
    */
-  public function __construct(KeyValueStoreExpirableInterface $storage, LockBackendInterface $lockBackend, $owner) {
+  public function __construct(KeyValueStoreExpirableInterface $storage, LockBackendInterface $lockBackend, $owner, $expire) {

    Can we also document it, please? I wonder whether we could make $expire optional on the constructor as well 

  2. +++ b/core/modules/user/lib/Drupal/user/TempStoreFactory.php
@@ -47,11 +54,14 @@ class TempStoreFactory {
+   *   The

    This is Nihilism

damiankloip’s picture

damiankloip commented
StatusFileSize
new 2273385-4.patch8.26 KB
new interdiff-2273385-4.txt1.45 KB

Rerolled for PSR-4, and comments above. Thanks!

damiankloip’s picture

damiankloip commented
StatusFileSize
new 2273385-5.patch4.09 KB

Oops, sorry. Accidentally included patch file. interdiff is still good.

damiankloip’s picture

damiankloip commented

5: 2273385-5.patch queued for re-testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Still nice! Maybe we want to use a change notice thingy.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 2273385-5.patch, failed testing.

Status: Needs work » Needs review

damiankloip queued 5: 2273385-5.patch for re-testing.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC. I guess we want also a chance record/notice thing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Regarding change record - let's update https://drupal.org/node/2095771 - less to read and all relevant info in one place :)

Committed f0d8d3f and pushed to 8.x. Thanks!

  • alexpott committed f0d8d3f on 8.x 
    Issue #2273385 by damiankloip: Use a container parameter for the user...
damiankloip’s picture

damiankloip commented

Was that the right link? I think that is your test issue :)

Status: Fixed » Closed (fixed)

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