Skip to content

The migration for setting the 'recurrent' property to 0 for all events is not ran automatically upon upgrade #79

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

Closed
lucaa opened this issue Jan 25, 2022 · 6 comments
Assignees
Milestone

Comments

@lucaa
Copy link

lucaa commented Jan 25, 2022

I reproduced this with an upgrade from version 2.9.3 to version 2.11-rc-1 , on XWiki 13.4.5, on a subwiki (actually, I have the case on more than a single subwiki, but the idea is that it's happened on a subwiki and not on the main wiki).
On my setup, this extension is a dependency of another extension, so it gets upgraded as a dependency, not as a toplevel extension (not sure if it's relevant or not, but just in case it is).

Expected result:

  • the migration of events in MoccaCalendar.Code.RecurrentEventMigration shows 0 events to migrate
  • the events in the database have value 0 for the recurrent property

Actual result

  • the migration of events in MoccaCalendar.Code.RecurrentEventMigration shows 12 events to migrate
  • in the database, all the events that that have value 0 for the recurrent property are the events created after the upgrade. The others value NULL value stored.
@lucaa lucaa added this to the 2.11 milestone Jan 25, 2022
@lucaa
Copy link
Author

lucaa commented Jan 25, 2022

It appears, from my checks, that the events that haven't been migrated properly (have NULL in the database), are not displayed on the calendar(s), for example they don't appear at all on the calendar displayed on the homepage on MoccaCalendar.WebHome .

@ClemensRobbenhaar ClemensRobbenhaar self-assigned this Jan 26, 2022
ClemensRobbenhaar added a commit that referenced this issue Jan 26, 2022
- fix the actual name of the extension
- add log statement (to be removed later)
  about which events arrive to be handled
@lucaa
Copy link
Author

lucaa commented Apr 26, 2022

I upgraded to version 2.11.1-rc1 of the calendar application, which is supposed to have this issue fixed, and I still have some events with NULL in the database and not displayed on calendars, and appearing counted in MoccaCalendar.Code.RecurrentEventMigration .

However, this time it's only part of the events that didn't get migrated. More precisely, I have multiple wikis on my instance:

  • in one of them, all events were properly migrated. There is an entry in their history saying "added recurrent=false", whose date and time corresponds to the date and time of the upgrade of the extension.
    • On this wiki, the history of the MoccaCalendarEventClass shows an install of 2.9.2, upgrade to 2.10 and then upgrade to 2.11.1-rc1 (current version)
  • in another one of them, there are still events with NULL value in the database for the recurrent property. On this other wiki there is no entry in the history of the events pages from the time of the installation of version 2.11.1-rc1 .
    • On this wiki, the history of the MoccaCalendarEventClass shows an install of 2.9.3, upgrade to 2.10 and then upgrade to 2.10.1-SNAPSHOT (installed on 10th of december 2021 at midnight - start of the day) but I don't know from when the snapshot dates.
      • there is no entry here for 2.11.1-rc1 because the class didn't change for that version, but the current version of the application on this wiki is 2.11.1-rc1
    • all events that are correct on this wiki are created after the installation of 2.10.1-SNAPSHOT
    • all events that are wrong on this wiki were created before the installation of 2.10.1-SNAPSHOT, and they got an entry in their history at the time of the install of 2.10.1-SNAPSHOT, saying Migrated property [recurrent] from class [MoccaCalendar.MoccaCalendarEventClass] , but that's it.

I checked the migration code and it seems to cover for the case when the property already exists but its value is NULL and it should be setting it to 0.

Questions:

  • is there any check in the migration code that would explain why no migration was ran when upgrading to 2.11.1-rc1 ? However, I can't say for sure it was from 2.10.1-SNAPSHOT or from another version, as I did many tests on this wiki
  • I saw in the code that the event the migration code reacts to is ExtensionUpgradedEvent. Would this also cover an uninstall of the old version (without removing the content pages) followed by an install of the newer version? Could it be that this is what I did on my wiki and that's why it didn't work - although there's little reason for this to have happened...

@ClemensRobbenhaar
Copy link
Collaborator

That the entries created after installing some 2.10.1-SNAPSHOT have the "recurrent" property is likely due to the fact that they are created after the event template got that property added. I remember that the upgrader of the old entries failed to run because of a typo until fixed.

It might be that the 2.11.1-rc1 version was also affected by this bug and skipped the migration. There is no diagnostics for this as the code does nothing if the UpgradeEvent is for another extension (it might otherwise create lots of noise e.g. on platform upgrade), and the typo caused it to think that its own upgrade was for another extension.

If this is still a problem after installing 2.11.1 (which should really try to run the upgrader), then one option is to downgrade to an older version and upgrade to 2.11.1 again. I have not tested if this really works, however.
Alternatively one can run the upgrader manually via the .../MoccaCalendar/Code/RecurrentEventMigration page. That should be a viable solution if it only affects a few sub wikis.

About uninstalling and installing a newer version: this will indeed skip/bypass the upgrader. I have not thought about that migration path. Running the manual upgrade is hopefully an option in that case for the current release. If it is not I need to add the ExtensionInstalledEvent to the list of events to listen to and then we need to make a small bugfix release for that change.

@lucaa
Copy link
Author

lucaa commented Apr 27, 2022

It might be that the 2.11.1-rc1 version was also affected by this bug and skipped the migration.

oh, right, there is this that needs clarifying.

The current issue is closed in 2.11 and thus before 2.11.1-rc1 .

Also, from the code history at the current version (2.11.1) https://github.com/xwikisas/application-mocca-calendar/commits/application-mocca-calendar-2.11.1 there should be no newer and better version of this migration. So if there's a bug for 2.11.1-rc1 , there is a bug in the current version, this is my logic and this is why I am even mentioning this here, to fix it if there's anything to fix.

That the entries created after installing some 2.10.1-SNAPSHOT have the "recurrent" property is likely due to the fact that they are created after the event template got that property added. I remember that the upgrader of the old entries failed to run because of a typo until fixed.

yes, that's fine, that part I had understood.

If this is still a problem after installing 2.11.1 (

I can try that just for debugging reasons, but as I mentioned above, to me there is no reason for 2.11.1 to be in any way better than 2.11.1-rc1. What I fear is that this may be a sporadic error - which it seems to be since it worked fine for other subwikis - and it may just happen to not reproduce for 2.11.1 and we'd conclude from it that 2.11.1 is somehow better...

@ClemensRobbenhaar
Copy link
Collaborator

I did not look carefully enough at the version when the issue should have been fixed. Indeed it there are still unmigrated calendar entries after installing 2.11.1-rc1 this looks like a bug.

Does the manual migrator at .../MoccaCalendar/Code/RecurrentEventMigration show that there are events left to be migrated? If no, then there is something wrong in the way the events to be migrated are determined. If yes, then the migrator did not run, possibly due to the uninstall/reinstall way to upgrade.

@lucaa
Copy link
Author

lucaa commented Apr 27, 2022

Does the manual migrator at .../MoccaCalendar/Code/RecurrentEventMigration show that there are events left to be migrated?

They are displayed as left to be migrated, so the detection is probably good.

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

No branches or pull requests

2 participants