Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce SessionService and SessionEntityInterface. #3555

Merged
merged 4 commits into from Apr 4, 2024

Conversation

demiankatz
Copy link
Member

@demiankatz demiankatz commented Apr 3, 2024

This PR sets up another database entity interface and service to pave the way for #2233. It makes some minor improvements upon the current code in #2233 (e.g. renaming getBySessionId to getSessionById for consistency with existing methods like getUserById in other services); I will reconcile the differences in #2233 after this is merged.

This also adds a unit test for the SystemStatus Ajax handler, which did not previously have one.

@demiankatz demiankatz added the architecture pull requests that involve significant refactoring / architectural changes label Apr 3, 2024
@demiankatz demiankatz added this to the 10.0 milestone Apr 3, 2024
@demiankatz demiankatz requested a review from aleksip April 3, 2024 20:37
Copy link
Contributor

@aleksip aleksip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor things, but otherwise looks good!

module/VuFind/src/VuFind/Db/Row/Session.php Outdated Show resolved Hide resolved
module/VuFind/src/VuFind/Db/Service/SessionService.php Outdated Show resolved Hide resolved
module/VuFind/src/VuFind/Db/Service/SessionService.php Outdated Show resolved Hide resolved
@demiankatz
Copy link
Member Author

Thanks for all the catches, @aleksip -- it shows that I was rushing to get this done at the end of the day; I appreciate your attention to detail. :-) I'm a little surprised that some of these things failed to trigger code style or phpstan warnings! In any case, I believe all of your suggestions have been addressed now.

@demiankatz demiankatz requested a review from aleksip April 4, 2024 11:08
Copy link
Contributor

@aleksip aleksip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@demiankatz Yes, I too was a bit surprised about that. Anyway, looks all good now!

@demiankatz demiankatz merged commit 7f3f684 into vufind-org:dev Apr 4, 2024
7 checks passed
@demiankatz demiankatz deleted the session-db-service branch April 4, 2024 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture pull requests that involve significant refactoring / architectural changes
Projects
None yet
2 participants