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 support #277

Merged
merged 9 commits into from
May 2, 2024
Merged

Conversation

jacksleight
Copy link
Contributor

@jacksleight jacksleight commented Apr 26, 2024

Adds token support so live preview tokens are stored in the database.

There are a couple of things that are out of the ordinary:

  • There's no TokenRepository contract, so we're just binding to the file repository class instead.
  • The file repository make method doesn't use the service container, so we have to override it.
  • Calling app(Token::class)::fromModel($model) doesn't work due to the constructor arguments, so this just calls Token::fromModel($model) directly.

These probably require PRs to core if we want them to work like everything else.

Let me know if anything needs changing.

The new update script assumes this will be in 3.5.0.

@sylvesterdamgaard
Copy link
Contributor

@ryanmitchell PR for supporting the preview tokens as we talked about. :)

@ryanmitchell
Copy link
Contributor

ryanmitchell commented Apr 26, 2024

Awesome, thanks for this.

I've added a PR to core for the cli install support: statamic/cms#9962

We're switching to default repositories file rather than eloquent from the next release, so I've gone ahead and changed that in advance of that. You can just run the migration manually anyway, and opt-in (which is what I would expect on our installs).

@ryanmitchell ryanmitchell changed the base branch from master to statamic-5 April 26, 2024 12:08
@jasonvarga
Copy link
Member

There's no TokenRepository contract, so we're just binding to the file repository class instead.

This was dumb on my part. Since this PR is targeting Statamic 5, we can get a contract into core that you can bind to.

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

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

Once statamic/cms#9964 is merged, you should be able to do the mentioned changed.

Also, you'll probably need to adjust typehints.

src/Tokens/TokenRepository.php Outdated Show resolved Hide resolved
src/Tokens/TokenRepository.php Outdated Show resolved Hide resolved
src/Tokens/Token.php Outdated Show resolved Hide resolved
src/Tokens/Token.php Outdated Show resolved Hide resolved
@jacksleight
Copy link
Contributor Author

Cool, will do. Thanks!

@ryanmitchell
Copy link
Contributor

All looks good to me, but I'll let Jason merge it as he was working with you on this.

@duncanmcclean duncanmcclean mentioned this pull request May 1, 2024
2 tasks
@jasonvarga jasonvarga merged commit 0a08a75 into statamic:statamic-5 May 2, 2024
11 checks passed
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

Successfully merging this pull request may close these issues.

5 participants