Core-ify "Random Keywords" plugin

[dgw]

This is still WIP, don't merge yet. In particular, the check desired in #1877 for whether the standalone plugin is active doesn't do anything/might even crash the whole plugin (I've not tested it yet).

Mostly I'm looking for feedback on whether or not it's worth keeping the commit history of the original plugin. I did this with a merge using --allow-unrelated-histories. For the final PR, I can either prefix the merged-in commits with something so it's clear they pertain to the plugin, not YOURLS itself, or just remove the history and do a copy-paste import instead. What say you @ozh @LeoColomb?

Whatever the way forward, it's clear that having this functionality as a core plugin will (hopefully) reduce the number of issues/comments asking how to avoid sequential shorturls.

[ozh]

I for one really don't care about the git log and commit message history :) (I mean, nicely put commit messages are nice to have, but they shouldn't dictate the way one works)

[dgw]

@LeoColomb I don't exactly remember what I still wanted to do with this. There was something, but I don't think I have time to look from a computer until tomorrow.

[LeoColomb]

@dgw #1877 (comment)

(this would require a renaming of the functions and a check for previous version activated)

😉

[dgw]

Oh yeah. Maybe also a real option instead of $ozh_random_shorturl['length'] = 5; that needs to be edited in the file, since upgrading YOURLS would overwrite changes to that.

[dgw]

So I had a bit of time tonight after all. There's already a "conflict preventer" function that checks if the old plugin is active. It might not actually prevent loading this one in that case; I haven't had a chance to test it. (Honestly, I've been unable to find much useful documentation about how hooks are supposed to work. Do I just return false?)

But I noticed there was a merged PR in the other repo and pulled in those changes (tiny one-liner), and implemented a settings page which should work. 🤞

[ozh]

Yep, saw this PR, will have time to look at it tomorrow :)

[ozh]

Nope, won't work ...

There's a missing ( on line 40 : if yourls_is_active_plugin( 'random-keywords' ) should be if (yourls_is_active_plugin( 'random-keywords' )) 😄

Plus, I think checking with yourls_is_active_plugin() isn't foolproof enough: if someone has installed the plugin in directory 'random-keyword(no trailing s) orrandom-url-like-bitly`, will this work? I'm not sure how this works but I think it's based on the directory name.
I think a more foolproof way would be to check if function ozh_random_keyword() already exists

Also, it's hooked onto 'activated_random-shorturls' so I guess it will echo the warning after activation anyway, and you'll end up with both plugins enabled. I don't think we have something early enough before plugin activation

Maybe a simple way would be something like:

if (function_exists('ozh_random_keyword')) {
   yourls_add_notice('deactivate your plugin, it's obsolete') or something like this
} else {
    all the yourls_add_action and filter to actually register the newer plugin
} 

Thoughts ?

[dgw]

I was going to actually do some of this now, but I've run out of time. 😔 So it'll get done during the next block of free time I (think I) have.

Also, it's hooked onto 'activated_random-shorturls' so I guess it will echo the warning after activation anyway, and you'll end up with both plugins enabled.

The idea was to block activating the plugin entirely, but it looks like there's no hook capable of doing that. All of the plugin activation hooks run after the activation itself is already finalized. So, I'll have to overwrite that (stub) implementation with a different approach—probably based on detecting the old plugin's function name, yes.

There's a missing ( on line 40 : if yourls_is_active_plugin( 'random-keywords' ) should be if (yourls_is_active_plugin( 'random-keywords' )) 😄

I'm spending too much time in Python these days… 🤣 But replacing the conflict preventer as discussed above will eliminate this error anyway.

Code is nowhere close to ready, so having an approval on it is very misleading!

[dgw]

There, @ozh, I think that's more like what you were suggesting. Better?

[ozh]

Hmm, still doesn't work: having old one activated and activating new one, I get the expected notice but the plugin is still activated :

Random ShortURLs plugin cannot function unless Random Keywords is removed first.
Plugin has been activated

Will look into it

[ozh]

Meh. I'm dumb. Works completely as expected. :-)

[ghost]

@ozh Would it be possible to push a new release (e.g. v1.7.5) which includes this commit? It's been almost four months.

[ozh]

Totally. Will do this week-end

[OpNop]

Any update on this? I just installed YOURLS and am liking it, but just noticed that random urls wasn't there and the docs say to enable the bundled plugin (that's not in the 1.7.4 release)

[ozh]

Yeah, sorry for my lack of reaction, I'm awfully short on free time :)
Released! https://github.com/YOURLS/YOURLS/releases/tag/1.7.5

[ghost]

@ozh Thanks! How long does it typically take for new versions to appear on YOURLS' Docker Hub page?

[LeoColomb]

@whalehub It tipycally takes two days. It's done almost automatically, but some manual steps are required from outside of YOURLS team.