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

[WIP] Configurable Post Routes #171

Conversation

tomschlick
Copy link
Contributor

@tomschlick tomschlick commented Sep 18, 2016

As described in #167, this PR allows you to configure how post URLs are structured. With this you can do so in 2 steps.

The first is to change which url parameters are required in config/blog.php. You can do so by uncommenting the parameters you want in your url.

    'post_params' => [
//        'id',
        'slug',
        'year',
        'month',
        'day',
    ],

Second you'll want to change the blog.post.show route to include those parameters:

// Blog Post Page
Route::get('{year}/{month}/{day}/{slug}', 'Frontend\BlogController@showPost')->name('blog.post.show');

From there it should just work.

Let me know what you think about this approach. Obviously there are some issues with upgrading in the future but I think we can address those using the approach Laravel Spark is taking as we talked about in #95

fixes #167

@austintoddj
Copy link
Owner

@tomschlick I really like this feature. If you're confident we can address any upgrade issues with the upgrade process you're working on, then I've got no problem merging this in.

Quick question: The Settings page in the admin section has really begun to house all the blog config options. Since I want us to also be able to cater to the non-Laravel/new developers, is there a way we can provide this option from the GUI? Perhaps specifying the path in the settings form? I think if we can provide a blog where 99% of the configuration can be done from the GUI, it'll look far less intimidating to someone who just tries it out. Does that make sense?

@tomschlick
Copy link
Contributor Author

We could definitely stash the config in the db and have that trigger what fields are required... the issue is with the blog post routes itself. Maybe this is a good case for a catchall? Not sure what the solution is there... any ideas?

@tomschlick
Copy link
Contributor Author

And yeah I'll outline my thoughts on the upgrade process in #95 later tonight or tomorrow

@austintoddj
Copy link
Owner

austintoddj commented Sep 19, 2016

I like that, storing the config options in the db. What were you referring to as a catch all? A route? Haven't thought of it before, what would that look like? @tomschlick

@tomschlick
Copy link
Contributor Author

So the catch-all would wait until all the other routes are checked and then the router would call some custom method like BlogController@routeForPost. From there that method would grab the url, compare it to the posts on file and decide if it should display a post or 404.

That seems like the way to go but we would have to basically duplicate the routing logic to check for those in the custom method. I'm looking into how a few other Laravel based CMSs have done it.

@austintoddj
Copy link
Owner

@tomschlick That sounds like it'd be good. Haven't ever seen that done before, but it sounds like the right direction for this.

@tomschlick
Copy link
Contributor Author

The best way I can think to do it right now is to add a uri field to the posts table. Upon publish of a post it would generate the uri for that post automatically. If you change the slug/publish date/etc it would auto regenerate.

From there it would be trivial to do a db lookup on the posts table during a request.

The alternative is to somehow parse the URL against the structure in the config such as {year}/{month}/{day}/{slug} and do queries based on that but it seems like a lot of work for little gain.

Thoughts?

@austintoddj
Copy link
Owner

@tomschlick Never even thought about that approach, but I like it. That seems like a good way to progress with this. You've got my thumbs up for it.

@tomschlick
Copy link
Contributor Author

Ok, I'll look into amending this over the next day to use what we just talked about 👍

@austintoddj
Copy link
Owner

Awesome, thanks for your work on this. Really appreciate your contribution, you're making it a better app with each commit. 👍

@tomschlick
Copy link
Contributor Author

No problem! I can really see this taking off as the defacto laravel blogging standard. Some of the improvements I have suggested related to #95 could even allow it to be attached to an existing laravel application much like spark can be... but thats another ticket for another day 😉

@austintoddj
Copy link
Owner

Thanks for the kind words! I really look forward to what the community can do with this!

@austintoddj
Copy link
Owner

@tomschlick Did you want me to merge this PR in right away? Or was there anything else you were planning on doing with it or something else?

@tomschlick
Copy link
Contributor Author

Just hold off until I can amend the PR with those changes. I think those are a much better solution than what I currently have here

@tomschlick tomschlick changed the title Configurable Post Routes [WIP] Configurable Post Routes Sep 21, 2016
@abhishek619
Copy link

error in run

@tomschlick
Copy link
Contributor Author

Sorry for the delay in this. Might have some time over the next week or two to finish it up.

@austintoddj
Copy link
Owner

@tomschlick Sounds good, looking forward to it!

@austintoddj
Copy link
Owner

@tomschlick As a quick note, when you're looking into this I was wondering if you could look into another thing I wanted to tweak with all this.

Basically, I want to remove the /blog prefix on all the frontend routes. So everything from the backend can stay as is with the /admin prefix, but I don't like the look of seeing /blog in front of all the posts, tags etc. I think it'd be cleaner to have it all stemming off the root of the site. Does that make sense? I think that may have issues with the Login route however.

@tomschlick
Copy link
Contributor Author

Yeah the more I think about it, the more I'm inclined to say there should
be a separate "routes/urls" table that manages urls for anything in the
system.

Those can be versioned so previous urls redirect to the current ones. And
you could run them as a catch all after the main routes.php file has run so
there wouldn't be any conflicts between the system URLs.

Thoughts?

On Wed, Nov 16, 2016 at 2:29 PM Todd Austin notifications@github.com
wrote:

@tomschlick https://github.com/tomschlick As a quick note, when you're
looking into this I was wondering if you could look into another thing I
wanted to tweak with all this.

Basically, I want to remove the /blog prefix on all the frontend routes.
So everything from the backend can stay as is with the /admin prefix, but
I don't like the look of seeing /blog in front of all the posts, tags
etc. I think it'd be cleaner to have it all stemming off the root of the
site. Does that make sense? I think that may have issues with the Login
route however.


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#171 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAESKMUHQsIGfbqHVMC7fVjFnWeJDKCFks5q-1m1gaJpZM4KADRw
.

Sent from my iPhone

@austintoddj
Copy link
Owner

@tomschlick I like that idea a lot. Never thought about doing it that way, but if you're able to make that a feature, I'm all for it 100%.

@tomschlick
Copy link
Contributor Author

Yeah that would also allow you to manage things like a pages module in the future.

@austintoddj
Copy link
Owner

Perfect! So you think it's pretty feasible to add that feature?

@Naoray
Copy link
Contributor

Naoray commented Nov 17, 2016

@austintoddj maybe we could also modify the xxx/admin route to something more generic, like xxx/dashboard? => because now many users can access the backend panels?!

@austintoddj
Copy link
Owner

@Naoray I think sticking with admin prefixing all the backend routes should be suitable for our purposes for now. Just taking a page from WordPress's playbook with all their backend routes being prefixed by wp-admin.

@austintoddj
Copy link
Owner

@tomschlick I believe a PR like this can now be migrated or re-created over in Easel, since that's the core code now. I'd still love to see this implemented, so if you want to close this out and attempt it over there I'd be very grateful!

@tomschlick
Copy link
Contributor Author

tomschlick commented Dec 1, 2016 via email

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.

[Suggestion] Ability to customize url format
4 participants