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

Add upgrade to re-run the comments upgrade to catch entitities possibly missed on first run due to Elgg core bug #3

Closed
iionly opened this issue Nov 16, 2014 · 5 comments

Comments

@iionly
Copy link
Owner

iionly commented Nov 16, 2014

Elgg core possibly hasn't converted all comment annotations to entities. With this core issue fixed in Elgg 1.9.X it's necessary to repeat at least part of tidypics/upgrades/2014040301.php.

To do:

  • wait until comment upgrade is fixed in Elgg core and then make this Elgg version the minimum version requirement,
  • evaluate tidypics/upgrades/2014040301.php to find out if any damage has been made by running the Tidypics upgrade prior the comments migration,
  • add a second Tidypics upgrade script to be run after the comments migration in Elgg core has been finished,
  • Possibly to block running the Tidypics upgrade if Elgg release indicates that comments migration has not been executed yet? Alternatively, maybe check for any comment annotations remaining and don't do the upgrade if there are any.
@juho-jaakkola
Copy link
Contributor

It seems that the plugin version number defines which upgrades should be ran. What if you just rename the latest upgrade file?

@iionly
Copy link
Owner Author

iionly commented Nov 16, 2014

That would be a possible (the easiest) solution I also have thought about already.

I think it's impossible to stop the Tidypics upgrade to be run by the admin even if he hasn't run the core upgrade yet.

What I'm mostly worried about is that running the script a second time might result in invalid modifications (due to new comments already made on Elgg 1.9 that shouldn't get touched).

@juho-jaakkola
Copy link
Contributor

Is it not possible to check whether the Tidypics upgrade has already been done to an individual ElggComment?

@iionly
Copy link
Owner Author

iionly commented Nov 16, 2014

I don't think it's possible to check if an entity (and also the corresponding river entry) has already been updated previously or not. Or maybe it's possible but then I guess it won't be any less time-consuming running the upgrade.

I think it's save to run the former upgrade a second time, so I've simply changed the upgrade script date and plugin version. Of course, everyone upgrading Tidypics needs to do the upgrade now but I see no other practical way. I also see no simple way of checking completeness of core upgrades to prevent the Tidypics upgrade getting run too soon (as I don't want to rewrite the whole upgrade script handling of Tidypics just for this single case).

Assuming that the core bug will be fixed in Elgg 1.9.5 I've already made a commit (Tidypics 1.9..4.1). If anyone (other than me) could also test it as soon as Elgg 1.9.5 is out, I would be quite grateful as I'm slightly worried that I might have missed some conditions that might result in the script not working as intended.

@iionly
Copy link
Owner Author

iionly commented Nov 17, 2014

Fixed with release 1.9.4.1.

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

No branches or pull requests

2 participants