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

Tokens for random number and random hash are missing #4672

Closed
olafgrabienski opened this issue Oct 1, 2020 · 18 comments · Fixed by backdrop/backdrop#4595
Closed

Tokens for random number and random hash are missing #4672

olafgrabienski opened this issue Oct 1, 2020 · 18 comments · Fixed by backdrop/backdrop#4595

Comments

@olafgrabienski
Copy link

olafgrabienski commented Oct 1, 2020

Description of the bug

Drupal 7 (with the Token module installed) provides "Random" tokens like [random:number]. Backdrop has integrated tokens but without the Random ones. They should be added to Backdrop, in my opinion.

The Drupal implementation of Random tokens

@docwilmot
Copy link
Contributor

Are those in D8 core? Would make a simple contrib module I'd think. Doubt its a commonly used feature.

@olafgrabienski
Copy link
Author

I find them useful but I guess a contrib module would also be okay. (For the time being, I have a custom one.) Not sure about the best contrib approach. One module per missing token seems odd. To integrate in core could be more easy and wouldn't do any harm, would it?

However, yesterday @klonos` suggested in Zulip:

..if these tokens were in the 7.x Token module and not in Backdrop, then they should be. Can you please open an issue for it if that's the case

https://backdrop.zulipchat.com/#narrow/stream/218635-Backdrop/topic/Random.20token/near/211789941

@catorghans
Copy link

I find them useful but I guess a contrib module would also be okay. (For the time being, I have a custom one.) Not sure about the best contrib approach. One module per missing token seems odd. To integrate in core could be more easy and wouldn't do any harm, would it?

However, yesterday @klonos` suggested in Zulip:

..if these tokens were in the 7.x Token module and not in Backdrop, then they should be. Can you please open an issue for it if that's the case

https://backdrop.zulipchat.com/#narrow/stream/218635-Backdrop/topic/Random.20token/near/211789941

Can you share your custom one?

@olafgrabienski
Copy link
Author

Can you share your custom one?

Sure (attached below), hope it helps!

custom_random_tokens.zip

@indigoxela
Copy link
Member

indigoxela commented May 12, 2022

Are those in D8 core? Would make a simple contrib module I'd think.

@docwilmot I did a quick check on D9 - random tokens don't ship with core, but with a contrib module. So we might do the same or get it into Backdrop core for BC.

One module per missing token seems odd

@olafgrabienski I agree. Does anyone here know, which other tokens (types) aren't in core, but shipped with Token in D7.

If it's really only the random tokens, I'd recommend to put them in core (that's just a few lines of code). If there are several missing, we could combine them in a contrib module - if we agree on that.

You might have guessed - I'm using random tokens in a project and if they're in core, the upgrade might get easier.

@stpaultim stpaultim added this to To do in D7 upgrade path via automation Sep 22, 2022
@nattywebdev
Copy link

Would be great to have this custom module as a contrib module, at least so I can remember where to find it next time I need it! Thank you @olafgrabienski

@olafgrabienski
Copy link
Author

olafgrabienski commented Dec 5, 2023

Would be great to have this custom module as a contrib module

At the moment, I don't have the time to publish the custom module as a contributed one, and I still think random tokens should be integrated into core (for convenience, and for easier upgrades from Drupal 7). That said, I wouldn't object if anyone else published it as contrib module.

@kiamlaluno
Copy link
Member

I agree with @olafgrabienski: At least for sites migrating from Drupal 7, that token could be helpful.

It is also true those tokens are defined by a contributed Drupal 7 module, which means only the Drupal 7 sites with the Token module rely on those tokens. Since the Token module in BackDrop is part of core, those tokens should be defined in core.

@indigoxela
Copy link
Member

My interpretation of recent comments here is, that adding random tokens to core is preferred.

Here's a PR that provides that. Probably the easiest way to test is to use the PR sandbox, and play with node alias patterns.

@nattywebdev
Copy link

Thank you @indigoxela - I've done a little testing and recorded it as comments on your PR.
If this feature can be included in core it would be marvellous - not to mention the time-saving in hunting down this issue!

@indigoxela
Copy link
Member

@nattywebdev many thanks for testing, I'm setting labels accordingly (assuming that you don't have permission to do so - or, do you?).

@argiepiano
Copy link

Thank you for providing a PR, @indigoxela, and for testing, @nattywebdev. I've reviewed the code, and it LGTM. This is RTBC.

@nattywebdev
Copy link

@nattywebdev many thanks for testing, I'm setting labels accordingly (assuming that you don't have permission to do so - or, do you?).

Absolutely not - I'm too ignorant to be let loose on anything important!

@olafgrabienski
Copy link
Author

olafgrabienski commented Dec 7, 2023

@indigoxela Many thanks for making the PR! While random tokens are available on the URL alias patterns page, I didn't find them on a content type configuration page. Seems it's not possible to use the new tokens in the content type config vertical tab "URL alias pattern".

@indigoxela
Copy link
Member

indigoxela commented Dec 7, 2023

I didn't find them on a content type configuration page

Nice catch, none of the global token types (types that need no additional info) are available there. Neither "Current date" nor "Current user"... Not sure, if fixing that should be part of this issue here.

In function path_form_node_type_form_alter (file core/modules/path/path.module) the available token_types are limited to only "node". So not a new thing, only discovered now.

@olafgrabienski
Copy link
Author

none of the global token types (...) are available there. Neither "Current date" nor "Current user"... Not sure, if fixing that should be part of this issue here.

I see, separate issue then, I guess!

D7 upgrade path automation moved this from To do to Done Dec 7, 2023
@quicksketch
Copy link
Member

Thanks folks for the renewed interest in this issue (and @olafgrabienski for filing it back in 2020). I agree with others that a missing token from D7 is a bug, as it could disrupt upgraded sites. The PR looks good to me too and matches the D7 implementation.

I merged backdrop/backdrop#4595 into 1.x and 1.26.x. Thanks everyone!

@klonos
Copy link
Member

klonos commented Dec 8, 2023

FTR: opened #6328 as a follow-up.

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

Successfully merging a pull request may close this issue.

9 participants