User tests: Successful: Unsuccessful:
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
True, was a bit in a rush and didn't saw that it caches the instances per client. Sounds like another factory in the container. Do we want to go that way?
I think just do it like the application in the container yeah? This ones kinda weird because there isn't an Admin Pathway (at least at the moment) - although with the admin menu changes in 3.8 it wouldn't totally surprise me if one came along at some point
Labels |
Added:
?
|
Modified the pr to get the pathway object from the container by a service provider. Changed the deprecate message to get the pathway from the app, as every core extension does already.
A little late to the party here, but...
Shouldn't there just be a $this->pathway
property on the application object? If it's supposed to be persistent for the lifetime of the request, the pathway object should be very closely bound to the active application and should live as a property there instead of always being reliant on fetching the object out of the container (we can still use the container to build the initial service, and we should drop the unused options param from the constructor as well).
Added your input.
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-02-11 12:13:40 |
Closed_By | ⇒ | wilsonge |
I'm unsure about this - the pathway is constant for a given page - we shouldn't load this multiple times. If we aren't doing a
getInstance
it at least needs to be in the container and retrieved consistently through the container. We shouldn't be recommending creating a fresh instance each time