- Issue created by @fago
- 🇸🇮Slovenia useernamee Ljubljana
I guess the way forward would be to make RestLogSubscriber::isRestPage method easily extensible from outside with SOLID principles and rename it to sth like
applies
. - 🇸🇮Slovenia useernamee Ljubljana
I propose to add voter design pattern to determine whether to log an request or not.
- Assigned to ad0z
- Issue was unassigned.
- Status changed to Needs review
8 months ago 8:05pm 13 April 2024 - 🇵🇱Poland ad0z
Okay, so I've added
RouteCheckManager
and replacedisRestPage
, it should be quite easy to extend logging mechanism if needed now.
@fago and @useernamee can you guys review issue's fork, give feedback and let me know if we need more work to make Lupus integration happen.. - 🇸🇮Slovenia useernamee Ljubljana
Thank you @ad0z for improvements, I checked PR and it looks good to me. I've created a Lupus Decoupled Drupal issue to implement it 📌 Improve debugging/developer experience with rest_log module Needs review . I'll move this ticket into RTBC after I test it with Lupus Decoupled Drupal.
- Merge request !12Resolve #3432400 "Add support to log other decoupled routes." → (Merged) created by useernamee
- Status changed to RTBC
8 months ago 12:09pm 22 April 2024 - 🇸🇮Slovenia useernamee Ljubljana
I've created a MR on 📌 Improve debugging/developer experience with rest_log module Needs review and tested the issue locally. It worked well. I did also create a MR.
I did not really need the currentRouteMatch in
check
in our log check (https://git.drupalcode.org/project/lupus_decoupled/-/merge_requests/83/d...) but I don't really see it as an issue. - Status changed to Needs review
8 months ago 5:01pm 22 April 2024 - 🇵🇱Poland ad0z
@useernamee I removed currentRouteMatch injection from RestLogSubscriber and moved it to the RestPageRouteCheck service, I think it makes more sense like that, as it was injected in RestLogSubscriber just for the purpose of RestPageRouteCheck. So, I updated it - I think it's better to do it now than later. I hope you don't mind it, please update your code as well by removing current route match from parameters. If you mind I can revert last commit commits.. but hope not :)
- Status changed to RTBC
8 months ago 3:43pm 23 April 2024 -
ad0z →
committed 00efabd4 on 2.x authored by
useernamee →
Issue #3432400: Add support for Lupus Decoupled Drupal
-
ad0z →
committed 00efabd4 on 2.x authored by
useernamee →
- Status changed to Fixed
8 months ago 6:08pm 23 April 2024 Automatically closed - issue fixed for 2 weeks with no activity.