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

Configurable user model & new aggregate value #34

Merged
merged 5 commits into from
May 7, 2024

Conversation

DevHoracioRodriguez
Copy link
Contributor

@DevHoracioRodriguez DevHoracioRodriguez commented Apr 30, 2024

Description

Making user model configurable and add SECOND aggregate to set microsecond to 0.

Does this close any currently open issues?

No. But the configurable model is needed to make code compatible with LdapRecord. LdapRecord, when using database configuration, the user relation is from auth.providers.users.database.model and not from auth.providers.users.model. Therefore, both packages won't work together. As of now, a custom model is required.

The aggregate is needed for MSSQL. Currently, once date is inserted, the datetime value still accepting time other than 0.

Before aggregate: 2024-04-29 00:00:00.950
After aggregate: 2024-04-30 00:00:00.000

Adding SECOND as aggregate option, and adding User Model configuration variable.
1. Making user relation configurable.

2. Adding SECOND aggregate to set microsecond to 0
MSSQL isn't getting microseconds as 0.
- Value before changes: 2024-04-29 00:00:00.950

New code now set microsecond to 0.
- Value after changes: 2024-04-30 00:00:00.000

Note: Migrate haven't been modified. Date column type is datetime.
@DevHoracioRodriguez
Copy link
Contributor Author

In our side, now the counter updates as expected.

bilfeldt added 3 commits May 7, 2024 22:19
Remove part from separate PR
Revert relationship changes
Copy link
Owner

@bilfeldt bilfeldt left a comment

Choose a reason for hiding this comment

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

Thanks @CyberEkklesiaOwner 👍

@bilfeldt bilfeldt merged commit 33b6ee5 into bilfeldt:main May 7, 2024
11 checks passed
@DevHoracioRodriguez
Copy link
Contributor Author

DevHoracioRodriguez commented May 8, 2024

Hello @bilfeldt

Thank you for accepting some changes.

Is there anything wrong with the relationship changes? route-statistics.php & RouteStatistic.php updates were removed.

Could those changes be added to make user model configurable? At least similar to logger...

@bilfeldt
Copy link
Owner

bilfeldt commented May 8, 2024

@CyberEkklesiaOwner sorry about the confusion.

Can you please make a separate PR for the user model config, then I will happily merge that.

@DevHoracioRodriguez
Copy link
Contributor Author

@bilfeldt, no worries!

Please, review new PR #35

Thank you for the opportunity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants