Skip to content

Migrate annotations to a separate db table, update model, update tests#23564

Merged
michalkleiner merged 93 commits into
5.x-devfrom
dev-19428
Sep 24, 2025
Merged

Migrate annotations to a separate db table, update model, update tests#23564
michalkleiner merged 93 commits into
5.x-devfrom
dev-19428

Conversation

@michalkleiner

Copy link
Copy Markdown
Contributor

Description:

  • add common helper to flatten arrays with a unit test
  • add migration from option table to annotations table using two-part custom migration
  • updates the Annotations plugin with a new model, which makes use of the new annotations table to correctly read and write annotations.

Note: could probably do without the array flattening helper but it feels it made the code a bit more legible in terms of what belongs to each migration.

Review

michalkleiner and others added 30 commits August 25, 2025 23:10
* Create a model for annotations in a separate db table

* Add index in a separate db call

* Remove private var table until needed

* Only create table and add index if needed

* Update UI test screenshot

* Update version to 5.4.1-b1 as the minor will go stable in the meantime

* Add user to new annotations db table

* Target version 5.5.0-b1

* Remove id from db table index

* Remove install and uninstall methods from model class

* Remove install/uninstall calls

* Update plugins/Annotations/Annotations.php

---------

Co-authored-by: Marc Neudert <marc@innocraft.com>
# Conflicts:
#	core/Db/Schema/Mysql.php
#	core/Version.php
#	plugins/Annotations/Model.php
…king

# Conflicts:
#	core/Updates/5.5.0-b1.php
# Conflicts:
#	core/Version.php
# Conflicts:
#	core/Version.php
@michalkleiner

Copy link
Copy Markdown
Contributor Author

@mneudert I think all the feedback has been addressed, can you please have another look? Thanks!

@michalkleiner michalkleiner requested a review from a team September 10, 2025 21:31
Comment thread plugins/Annotations/API.php
Comment thread plugins/Annotations/API.php Outdated
Comment thread plugins/Annotations/API.php Outdated
Comment thread plugins/Annotations/API.php
Comment thread plugins/Annotations/API.php Outdated
Comment thread plugins/Annotations/API.php Outdated
@michalkleiner michalkleiner requested a review from a team September 19, 2025 11:42
@caddoo

caddoo commented Sep 23, 2025

Copy link
Copy Markdown
Contributor

Happy that @mneudert 's concerns have been addressed.

Also happy with the system/integration tests covering most cases, this looks good to go.

caddoo
caddoo previously approved these changes Sep 23, 2025

@sgiehl sgiehl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The new test related changes look fine. So assuming the previous reviews were complete, this should now be ready to merge.

@michalkleiner michalkleiner dismissed mneudert’s stale review September 24, 2025 15:22

approved by Matt and Stefan

@michalkleiner michalkleiner merged commit cc873fc into 5.x-dev Sep 24, 2025
46 of 49 checks passed
@michalkleiner michalkleiner deleted the dev-19428 branch September 24, 2025 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: Security For issues that make Matomo more secure. Please report issues through HackerOne and not in Github. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc.

Development

Successfully merging this pull request may close these issues.

6 participants