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

FEATURE: add dimensions hash to node event model #3279

Conversation

andrehoffmann30
Copy link
Contributor

I want to extended the history view of the package https://github.com/aertmann/history with a filter for the dimension. With this website admins get a much needed additional filter for the history view of a web site with several dimensions and dimension values. In preparation for the extensions of the https://github.com/aertmann/history package I created these changes to the node event model. Please let me know, if additional changes are needed. Thank you in advanced.

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed

{
$eventLogResult = $this->connection->executeQuery('SELECT uid, dimension FROM neos_neos_eventlog_domain_model_event');

while ($eventLogInfo = $eventLogResult->fetchAssociative()) {
Copy link
Member

@daniellienert daniellienert Jan 27, 2021

Choose a reason for hiding this comment

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

This might need a loooong time for larger node event tables (and they tend to get pretty large). It thought about how to delegate more of the work to the database. What about:

--pseudo code--

while $dimensionsArray = 'SELECT dimension FROM neos_neos_eventlog_domain_model_event where dimensionshash IS NULL LIMIT 1' {
    $dimensionsHash = Utility::sortDimensionValueArrayAndReturnDimensionsHash($dimensionsArray);
    $this->connection->executeStatement('UPDATE neos_neos_eventlog_domain_model_event SET dimensionshash = ? WHERE dimension = ?', [$dimensionsHash, $dimensionsArray);
}

Should work in O(n), with n = dimensionCombinations and should be much faster.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @daniellienert , I adjusted the postUp function according to your suggestion. Can you please check, whether my adjustments are ok? Thank you.

@markusguenther
Copy link
Member

Just reopened to trigger the pipelines again :)

@markusguenther
Copy link
Member

Could you work on that for 8.3?

@andrehoffmann30
Copy link
Contributor Author

Hi @markusguenther, yes I will take a look at it and rebase my changes onto the 8.3 code base.

@andrehoffmann30 andrehoffmann30 force-pushed the feature/add-dimensions-hash-to-node-events branch from e23e191 to de8fe60 Compare February 19, 2023 14:11
@andrehoffmann30 andrehoffmann30 changed the base branch from 8.2 to 8.3 February 19, 2023 14:12
@andrehoffmann30
Copy link
Contributor Author

Hi @markusguenther I rebased my changes. Because I first rebased to the 8.2 branch and then to the 8.3 branch a tranalation from 8.2 is also in my changes which is missing in 8.3 at the moment. After 8.2 is upmerged into 8.3 this change should not be listed. Please check my changes and let me know, if something needs to be adjusted. Tnak you.

@markusguenther markusguenther force-pushed the feature/add-dimensions-hash-to-node-events branch from de8fe60 to 1975ee6 Compare March 31, 2023 09:58
Copy link
Member

@markusguenther markusguenther left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution :)

Maybe @daniellienert can add another review.

@markusguenther markusguenther merged commit 24bed8c into neos:8.3 Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

3 participants