Proposed Page Extension for Market & Location #85

[drewroberts]

• [x] requires updated drewroberts/blog package before merge

---

This PR allows both Market (#41) & Location (#42) to become extensions of the Page (https://github.com/drewroberts/blog/issues/15) model in drewroberts/blog and therefore simplifies the issues with routing we are running into with the following PR:

https://github.com/drewroberts/blog/pull/59

Here is the proposal for the overall routing structure that will be used for all Tipoff applications:

https://github.com/drewroberts/blog/issues/50

{page}/{child-page}/{grandchild-page}

In that example, Market will become the {page} and Location will become the {child-page} and therefore all Locations in the saving boot method must verify that the Page associated with the Market that the location belongs to is also the parent_id on the Page associated with the Location.

[pdbreen]

@drewroberts - this can work, but doesn't solve the issue with a package trying to register a route with 3 wildcard segments and no fixed segment to match on. Additional things this change will require:

• should creation of a market or location automatically create a corresponding page model based on market/location slug? Or, should the page model come first? If the former, should an error occur if a page already exists with the slug, or should the slug be modified to be unique?
• for non blog pages that still require market / location context, the bindings for {market} and {location} need to become custom bindings that match slug based on page slug (can be done a few different ways)
• remove the **/detail routes, controllers and views from this package

Since you seem determined to support paths with absolutely no fixed segment to match upon, I think you only have 2 options

• document the need to register the generic 3 segment wildcard route manually to ensure it will never steal any 3 segment routes from any other package or the app itself, and don't register it automatically within the package
• have the blog package register the actual, realized route paths that truly exist dynamically via a DB query (eg, if there are 2 page models in the DB with slug 'page-1' and 'page-2', the route paths registered in laravel are truly /page-1 and /page-2 -- not a model binding

If you go with the second solution, the blog package (which is registering the routes) needs to make sure that any new/deleted/changed page model slug will properly deal with route caching. This means -- figure out if routes are cached or not (either by disk inspection or config) and if they are, update the cache with the artisan command. This needs to happen on ALL servers - not just the server that happened to process the page model change. You also need to make sure permissions allow for the route:cache command to update the cache.

[drewroberts]

Thank you, @pdbreen. Those are great points. Let's discuss this one first:

Should creation of a market or location automatically create a corresponding page model based on market/location slug? Should an error occur if a page already exists with the slug, or should the slug be modified to be unique?

I like the concept of having it automatically create the page when you create a Market or Location. I'll add the slug back onto both resources and we'll need to lock pages that have a Market or Location relationship so that the Slug can only be updated on the resource in this package and upon saving, it will automatically update the Page slug.

Let's do both approaches in regards to the slug conflicts. Let's display an error in Nova if the slug already exists on another Page resource. For testing purposes with factories, it would be easiest to just create the function on the model that modifies it to be unique.

[pdbreen]

@wolfrednicolas - here a debugging tip you can use. Within any unit test, you can temporarily add $this->logToStderr()->logSql(); at the start of the test to see the SQL being run by the commands that come later. Just make sure to remove these lines prior to official commit.

[pdbreen]

@drewroberts - this is ready, but requires new version of drewroberts/blog before merge