? ? Pending

User tests: Successful: Unsuccessful:

avatar mbabker
mbabker
28 Jan 2019

Summary of Changes

#23433 was a bit hacky in creating the event listener for the session metadata manager. This moves it to a more correct architecture solution. Why does this matter? It allows the event to be properly tested without having to load a boatload of unrelated code is part of it. To accomplish this, the following changes are made:

  • Create a new LazyServiceEventListener, this is a decorator for a service from the DI container which needs to be lazy loaded at runtime (whereas event subscribers/listeners generally have to be instantiated when registered to the dispatcher, see old PR for why this is relevant)
  • Moves the event listener out from being a Closure in the Session service provider to its own class

Testing Instructions

  • Session metadata is still correctly processed, check database for this
avatar mbabker mbabker - open - 28 Jan 2019
avatar mbabker mbabker - change - 28 Jan 2019
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 28 Jan 2019
Category Libraries
avatar mbabker mbabker - change - 29 Jan 2019
Labels Added: ?
avatar mbabker
mbabker - comment - 19 Feb 2019

Nevermind, janky code it is.

avatar mbabker mbabker - change - 19 Feb 2019
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2019-02-19 01:54:32
Closed_By mbabker
avatar mbabker mbabker - close - 19 Feb 2019
avatar SharkyKZ
SharkyKZ - comment - 5 Mar 2019

Sir @mbabker could you re-open this if it's not too much trouble?

avatar mbabker mbabker - change - 5 Mar 2019
Status Closed New
Closed_Date 2019-02-19 01:54:32
Closed_By mbabker
avatar mbabker mbabker - change - 5 Mar 2019
Status New Pending
avatar mbabker mbabker - reopen - 5 Mar 2019
avatar SharkyKZ SharkyKZ - test_item - 6 Mar 2019 - Tested successfully
avatar SharkyKZ
SharkyKZ - comment - 6 Mar 2019

I have tested this item successfully on bb62224


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23708.

avatar SharkyKZ
SharkyKZ - comment - 6 Mar 2019

I have tested this item successfully on bb62224


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23708.

avatar SharkyKZ
SharkyKZ - comment - 6 Mar 2019

Not related to this PR but noticed that in J4 on first visit client_id value in session table is NULL. Is this a bug?

avatar joeforjoomla
joeforjoomla - comment - 6 Mar 2019

@SharkyKZ this was a 'bug' i reported some months ago, but in the current Alpha 7 this seems to be fixed. On first visit the client is now '0' and not NULL

avatar alikon alikon - test_item - 6 Mar 2019 - Tested successfully
avatar alikon
alikon - comment - 6 Mar 2019

I have tested this item successfully on bb62224


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23708.

avatar mbabker
mbabker - comment - 6 Mar 2019

IIRC part of the intent with #23433 was to fix that bug in the first place. Although looking again it looks like the wrong pseudo-constants are being checked here and this needs correcting.

avatar Quy Quy - change - 6 Mar 2019
Status Pending Ready to Commit
avatar Quy
Quy - comment - 6 Mar 2019

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/23708.

avatar wilsonge wilsonge - change - 7 Mar 2019
Labels Added: ?
avatar wilsonge
wilsonge - comment - 7 Mar 2019

PR for the issue michael mentioned #24120

avatar wilsonge wilsonge - change - 7 Mar 2019
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2019-03-07 19:53:10
Closed_By wilsonge
avatar wilsonge wilsonge - close - 7 Mar 2019
avatar wilsonge wilsonge - merge - 7 Mar 2019
avatar wilsonge
wilsonge - comment - 7 Mar 2019

Thanks!

Add a Comment

Login with GitHub to post a comment