diff --git a/resources/js/components/structures/PageTree.vue b/resources/js/components/structures/PageTree.vue index a28076e4474..cad57488eb1 100644 --- a/resources/js/components/structures/PageTree.vue +++ b/resources/js/components/structures/PageTree.vue @@ -191,7 +191,15 @@ export default { this.initialPages = this.pages; return response; }).catch(e => { - this.$toast.error(e.response ? e.response.data.message : __('Something went wrong')); + let message = e.response ? e.response.data.message : __('Something went wrong'); + + // For a validation error, show the first message from any field in the toast. + if (e.response && e.response.status === 422) { + const { errors } = e.response.data; + message = errors[Object.keys(errors)[0]][0]; + } + + this.$toast.error(message); return Promise.reject(e); }).finally(() => this.saving = false); }, diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php index 4cd5ae33921..debd42a7a23 100644 --- a/resources/lang/en/validation.php +++ b/resources/lang/en/validation.php @@ -128,6 +128,8 @@ 'duplicate_field_handle' => 'Field with a handle of :handle cannot be used more than once.', 'one_site_without_origin' => 'At least one site must not have an origin.', 'origin_cannot_be_disabled' => 'Cannot select a disabled origin.', + 'unique_uri' => 'This URI has already been taken.', + 'duplicate_uri' => 'Duplicate URI :value', /* |-------------------------------------------------------------------------- diff --git a/src/Contracts/Entries/EntryRepository.php b/src/Contracts/Entries/EntryRepository.php index 4b1b4746d3b..fbaf755aa69 100644 --- a/src/Contracts/Entries/EntryRepository.php +++ b/src/Contracts/Entries/EntryRepository.php @@ -14,6 +14,7 @@ public function find($id); public function findByUri(string $uri); + /** @deprecated */ public function findBySlug(string $slug, string $collection); public function make(); diff --git a/src/Contracts/Taxonomies/TermRepository.php b/src/Contracts/Taxonomies/TermRepository.php index 8220df3e608..cd454b7b62b 100644 --- a/src/Contracts/Taxonomies/TermRepository.php +++ b/src/Contracts/Taxonomies/TermRepository.php @@ -14,6 +14,7 @@ public function find($id); public function findByUri(string $uri); + /** @deprecated */ public function findBySlug(string $slug, string $collection); public function make(string $slug = null); diff --git a/src/Data/ExistsAsFile.php b/src/Data/ExistsAsFile.php index 9ede14e8eaa..7a549641c01 100644 --- a/src/Data/ExistsAsFile.php +++ b/src/Data/ExistsAsFile.php @@ -13,6 +13,11 @@ trait ExistsAsFile abstract public function path(); + public function buildPath() + { + return $this->path(); + } + public function initialPath($path = null) { if (func_num_args() === 0) { @@ -77,9 +82,9 @@ public function fileExtension() return 'yaml'; } - public function writeFile() + public function writeFile($path = null) { - $path = $this->path(); + $path = $path ?? $this->buildPath(); $initial = $this->initialPath(); if ($initial && $path !== $initial) { diff --git a/src/Entries/Entry.php b/src/Entries/Entry.php index ab9cb9d1efb..c952bc0cc2c 100644 --- a/src/Entries/Entry.php +++ b/src/Entries/Entry.php @@ -335,6 +335,11 @@ public function taxonomize() } public function path() + { + return $this->initialPath ?? $this->buildPath(); + } + + public function buildPath() { $prefix = ''; diff --git a/src/Entries/GetDateFromPath.php b/src/Entries/GetDateFromPath.php index 47db536846b..a743983ea2b 100644 --- a/src/Entries/GetDateFromPath.php +++ b/src/Entries/GetDateFromPath.php @@ -8,8 +8,17 @@ public function __invoke($path) { $filename = pathinfo($path, PATHINFO_FILENAME); - return strpos($filename, '.') === false - ? null - : explode('.', pathinfo($path, PATHINFO_FILENAME))[0]; + if (strpos($filename, '.') === false) { + return null; + } + + $firstSegment = explode('.', pathinfo($path, PATHINFO_FILENAME), 2)[0]; + + return $this->isDate($firstSegment) ? $firstSegment : null; + } + + private function isDate($str) + { + return preg_match('/^\d{4}-\d{2}-\d{2}(-\d{4})?$/', $str); } } diff --git a/src/Entries/GetSlugFromPath.php b/src/Entries/GetSlugFromPath.php new file mode 100644 index 00000000000..219035ca732 --- /dev/null +++ b/src/Entries/GetSlugFromPath.php @@ -0,0 +1,28 @@ +isDate($segments[0])) { + return $segments[1]; + } + + return $segments[0]; + } + + private function isDate($str) + { + return preg_match('/^\d{4}-\d{2}-\d{2}(-\d{4})?$/', $str); + } +} diff --git a/src/Facades/Endpoint/Path.php b/src/Facades/Endpoint/Path.php index d4e60f19caa..bcd25442859 100644 --- a/src/Facades/Endpoint/Path.php +++ b/src/Facades/Endpoint/Path.php @@ -89,36 +89,6 @@ public function resolve($path) return $leadingSlash ? Str::ensureLeft($path, '/') : $path; } - /** - * Cleans up a given $path, removing any flags and order keys (date-based or number-based). - * - * Assumes the path will always end with an extension. - * - * @param string $path Path to clean - * @return string - */ - public function clean($path) - { - // Remove draft and hidden flags - $path = preg_replace('/\/_[_]?/', '/', $path); - - // Strip the order keys - $segments = explode('/', $path); - $total_segments = count($segments); - foreach ($segments as $i => &$segment) { - // Skip the final segment (the basename) if it doesn't contain two periods. - // This stops filenames like 404.md from being interpreted with 404 as - // the order key, resulting in a borked filename. - if ($i + 1 === $total_segments && substr_count($segment, '.') < 2) { - continue; - } - - $segment = preg_replace(Pattern::orderKey(), '', $segment); - } - - return implode('/', $segments); - } - /** * Assembles a URL from an ordered list of segments. * diff --git a/src/Facades/Endpoint/URL.php b/src/Facades/Endpoint/URL.php index b7d0a8f64ea..f8aca0b781e 100644 --- a/src/Facades/Endpoint/URL.php +++ b/src/Facades/Endpoint/URL.php @@ -245,27 +245,6 @@ public function getSiteUrl() return Str::ensureRight($rootUrl, '/'); } - /** - * Build a page URL from a path. - * - * @param string $path - * @return string - */ - public function buildFromPath($path) - { - $path = Path::makeRelative($path); - - $ext = pathinfo($path)['extension']; - - $path = Path::clean($path); - - $path = preg_replace('/^pages/', '', $path); - - $path = preg_replace('#\/(?:[a-z]+\.)?index\.'.$ext.'$#', '', $path); - - return Str::ensureLeft($path, '/'); - } - /** * Encode a URL. * diff --git a/src/Facades/Entry.php b/src/Facades/Entry.php index 73df8e89126..186f4f28f4d 100644 --- a/src/Facades/Entry.php +++ b/src/Facades/Entry.php @@ -11,7 +11,6 @@ * @method static \Statamic\Entries\EntryCollection whereInCollection(array $handles) * @method static null|\Statamic\Contracts\Entries\Entry find($id) * @method static null|\Statamic\Contracts\Entries\Entry findByUri(string $uri) - * @method static null|\Statamic\Contracts\Entries\Entry findBySlug(string $slug, string $collection) * @method static \Statamic\Contracts\Entries\Entry make() * @method static \Statamic\Contracts\Entries\QueryBuilder query() * @method static void save($entry) diff --git a/src/Facades/Term.php b/src/Facades/Term.php index 5b9b35e0bb9..b7c343ef87a 100644 --- a/src/Facades/Term.php +++ b/src/Facades/Term.php @@ -14,7 +14,6 @@ * @method static TermCollection whereInTaxonomy(array $handles) * @method static TermContract find($id) * @method static TermContract findByUri(string $uri, string $site = null) - * @method static TermContract findBySlug(string $slug, string $taxonomy) * @method static save($term) * @method static delete($term) * @method static TermQueryBuilder query() diff --git a/src/Facades/URL.php b/src/Facades/URL.php index 07d0957b944..a32c15e4464 100644 --- a/src/Facades/URL.php +++ b/src/Facades/URL.php @@ -20,7 +20,6 @@ * @method static string format($url) * @method static bool isExternal($url) * @method static string getSiteUrl() - * @method static string buildFromPath($path) * @method static string encode($url) * @method static mixed getDefaultUri($locale, $uri) * @method static string gravatar($email, $size = null) diff --git a/src/Http/Controllers/CP/Collections/CollectionStructureController.php b/src/Http/Controllers/CP/Collections/CollectionStructureController.php index 1cf75c4e1d7..1ffa78b57ce 100644 --- a/src/Http/Controllers/CP/Collections/CollectionStructureController.php +++ b/src/Http/Controllers/CP/Collections/CollectionStructureController.php @@ -3,6 +3,7 @@ namespace Statamic\Http\Controllers\CP\Collections; use Illuminate\Http\Request; +use Illuminate\Validation\ValidationException; use Statamic\Facades\Entry; use Statamic\Facades\User; use Statamic\Http\Controllers\CP\CpController; @@ -14,31 +15,22 @@ public function update(Request $request, $collection) { $this->authorize('reorder', $collection); - $deletedEntries = collect($request->deletedEntries ?? []) - ->map(function ($id) { - return Entry::find($id); - }) - ->filter(function ($entry) { - return User::current()->can('delete', $entry); - }); + $contents = $this->toTree($request->pages); - if ($request->deleteLocalizationBehavior === 'copy') { - $deletedEntries->each->detachLocalizations(); - } else { - $deletedEntries->each->deleteDescendants(); - } + $structure = $collection->structure(); + $tree = $structure->in($request->site); - $deletedEntries->each->delete(); + // Clone the tree and add the submitted contents into it so we can + // validate URI uniqueness without affecting the real object in memory. + $this->validateUniqueUris((clone $tree)->disableUriCache()->tree($contents)); - $tree = $this->toTree($request->pages); + // Validate the tree, which will add any missing entries or throw an exception + // if somehow the root would end up having child pages, which isn't allowed. + $contents = $structure->validateTree($contents, $request->site); - $tree = $collection->structure()->validateTree($tree, $request->site); + $this->deleteEntries($request); - $collection - ->structure() - ->in($request->site) - ->tree($tree) - ->save(); + $tree->tree($contents)->save(); } protected function toTree($items) @@ -52,4 +44,49 @@ protected function toTree($items) ]); })->all(); } + + private function validateUniqueUris($tree) + { + if (! $tree->collection()->route($tree->locale())) { + return; + } + + foreach ($tree->diff()->moved() as $id) { + $page = $tree->page($id); + $parent = $page->parent(); + + $siblings = (! $parent || $parent->isRoot()) + ? $tree->pages()->all()->slice(1) + : $page->parent()->pages()->all(); + + $siblings = $siblings->reject(function ($sibling) use ($id) { + return $sibling->reference() === $id; + }); + + $uris = $siblings->map->uri(); + + if ($uris->contains($uri = $page->uri())) { + throw ValidationException::withMessages(['uri' => trans('statamic::validation.duplicate_uri', ['value' => $uri])]); + } + } + } + + private function deleteEntries($request) + { + $deletedEntries = collect($request->deletedEntries ?? []) + ->map(function ($id) { + return Entry::find($id); + }) + ->filter(function ($entry) { + return User::current()->can('delete', $entry); + }); + + if ($request->deleteLocalizationBehavior === 'copy') { + $deletedEntries->each->detachLocalizations(); + } else { + $deletedEntries->each->deleteDescendants(); + } + + $deletedEntries->each->delete(); + } } diff --git a/src/Http/Controllers/CP/Collections/EntriesController.php b/src/Http/Controllers/CP/Collections/EntriesController.php index d81e5dc69f1..498c859a34f 100644 --- a/src/Http/Controllers/CP/Collections/EntriesController.php +++ b/src/Http/Controllers/CP/Collections/EntriesController.php @@ -3,6 +3,7 @@ namespace Statamic\Http\Controllers\CP\Collections; use Illuminate\Http\Request; +use Illuminate\Validation\ValidationException; use Statamic\Contracts\Entries\Entry as EntryContract; use Statamic\CP\Breadcrumbs; use Statamic\Facades\Asset; @@ -196,9 +197,9 @@ public function update(Request $request, $collection, $entry) } if ($collection->structure() && ! $collection->orderable()) { - $entry->afterSave(function ($entry) use ($parent) { - $tree = $entry->structure()->in($entry->locale()); + $tree = $entry->structure()->in($entry->locale()); + $entry->afterSave(function ($entry) use ($parent, $tree) { if ($parent && optional($tree->page($parent))->isRoot()) { $parent = null; } @@ -209,6 +210,8 @@ public function update(Request $request, $collection, $entry) }); } + $this->validateUniqueUri($entry, $tree ?? null, $parent ?? null); + if ($entry->revisionsEnabled() && $entry->published()) { $entry ->makeWorkingCopy() @@ -340,6 +343,8 @@ public function store(Request $request, $collection, $site) }); } + $this->validateUniqueUri($entry, $tree ?? null, $parent ?? null); + if ($entry->revisionsEnabled()) { $entry->store([ 'message' => $request->message, @@ -432,6 +437,40 @@ protected function formatDateForSaving($date) return $date; } + private function validateUniqueUri($entry, $tree, $parent) + { + if (! $uri = $this->entryUri($entry, $tree, $parent)) { + return; + } + + $existing = Entry::findByUri($uri); + + if (! $existing || $existing->id() === $entry->id()) { + return; + } + + throw ValidationException::withMessages(['slug' => __('statamic::validation.unique_uri')]); + } + + private function entryUri($entry, $tree, $parent) + { + if (! $tree) { + return $entry->uri(); + } + + $parent = $parent ? $tree->page($parent) : null; + + return app(\Statamic\Contracts\Routing\UrlBuilder::class) + ->content($entry) + ->merge([ + 'parent_uri' => $parent ? $parent->uri() : null, + 'slug' => $entry->slug(), + // 'depth' => '', // todo + 'is_root' => false, + ]) + ->build($entry->route()); + } + protected function breadcrumbs($collection) { return new Breadcrumbs([ diff --git a/src/Providers/ExtensionServiceProvider.php b/src/Providers/ExtensionServiceProvider.php index 2d28dbeb123..635caef7da5 100644 --- a/src/Providers/ExtensionServiceProvider.php +++ b/src/Providers/ExtensionServiceProvider.php @@ -186,6 +186,7 @@ class ExtensionServiceProvider extends ServiceProvider protected $updateScripts = [ Updates\AddPerEntryPermissions::class, Updates\UseDedicatedTrees::class, + Updates\AddUniqueSlugValidation::class, ]; public function register() diff --git a/src/Stache/Repositories/EntryRepository.php b/src/Stache/Repositories/EntryRepository.php index 02bbbe80760..4677239d40a 100644 --- a/src/Stache/Repositories/EntryRepository.php +++ b/src/Stache/Repositories/EntryRepository.php @@ -40,6 +40,7 @@ public function find($id): ?Entry return $this->query()->where('id', $id)->first(); } + /** @deprecated */ public function findBySlug(string $slug, string $collection): ?Entry { return $this->query() @@ -103,7 +104,7 @@ public function createRules($collection, $site) { return [ 'title' => 'required', - 'slug' => 'required|unique_entry_value:'.$collection->handle().',null,'.$site->handle(), + 'slug' => 'required', ]; } @@ -111,7 +112,7 @@ public function updateRules($collection, $entry) { return [ 'title' => 'required', - 'slug' => 'required|alpha_dash|unique_entry_value:'.$collection->handle().','.$entry->id().','.$entry->locale(), + 'slug' => 'required|alpha_dash', ]; } diff --git a/src/Stache/Repositories/TermRepository.php b/src/Stache/Repositories/TermRepository.php index 364374b1cc3..fdb659db32e 100644 --- a/src/Stache/Repositories/TermRepository.php +++ b/src/Stache/Repositories/TermRepository.php @@ -82,6 +82,7 @@ public function findByUri(string $uri, string $site = null): ?Term return $term->collection($collection); } + /** @deprecated */ public function findBySlug(string $slug, string $taxonomy): ?Term { return $this->query() diff --git a/src/Stache/Stores/CollectionEntriesStore.php b/src/Stache/Stores/CollectionEntriesStore.php index 58c6800baee..77972efe945 100644 --- a/src/Stache/Stores/CollectionEntriesStore.php +++ b/src/Stache/Stores/CollectionEntriesStore.php @@ -3,8 +3,10 @@ namespace Statamic\Stache\Stores; use Statamic\Entries\GetDateFromPath; +use Statamic\Entries\GetSlugFromPath; use Statamic\Facades\Collection; use Statamic\Facades\Entry; +use Statamic\Facades\File; use Statamic\Facades\Path; use Statamic\Facades\Site; use Statamic\Facades\YAML; @@ -62,7 +64,7 @@ public function makeItemFromFile($path, $contents) ->id($id) ->collection($collection); - $slug = pathinfo(Path::clean($path), PATHINFO_FILENAME); + $slug = (new GetSlugFromPath)($path); if ($origin = array_pull($data, 'origin')) { $entry->origin($origin); @@ -165,4 +167,36 @@ protected function storeIndexes() $collection->taxonomies()->map->handle() )->all(); } + + protected function writeItemToDisk($item) + { + $basePath = $item->buildPath(); + + if ($basePath !== $item->path()) { + return $item->writeFile($item->path()); + } + + $num = 0; + + while (true) { + $ext = '.'.$item->fileExtension(); + $filename = Str::before($basePath, $ext); + $suffix = $num ? ".$num" : ''; + $path = "{$filename}{$suffix}{$ext}"; + + if (! $contents = File::get($path)) { + break; + } + + $itemFromDisk = $this->makeItemFromFile($path, $contents); + + if ($item->id() == $itemFromDisk->id()) { + break; + } + + $num++; + } + + $item->writeFile($path); + } } diff --git a/src/Stache/Stores/TaxonomyTermsStore.php b/src/Stache/Stores/TaxonomyTermsStore.php index 3668b432805..049d6a510e0 100644 --- a/src/Stache/Stores/TaxonomyTermsStore.php +++ b/src/Stache/Stores/TaxonomyTermsStore.php @@ -4,8 +4,8 @@ use Facades\Statamic\Stache\Traverser; use Illuminate\Support\Facades\Cache; +use Statamic\Entries\GetSlugFromPath; use Statamic\Facades\File; -use Statamic\Facades\Path; use Statamic\Facades\Stache; use Statamic\Facades\Taxonomy; use Statamic\Facades\Term; @@ -51,7 +51,7 @@ public function makeItemFromFile($path, $contents) $term = Term::make() ->taxonomy($taxonomy) - ->slug(pathinfo(Path::clean($path), PATHINFO_FILENAME)) + ->slug((new GetSlugFromPath)($path)) ->initialPath($path) ->blueprint($data['blueprint'] ?? null); diff --git a/src/Structures/Page.php b/src/Structures/Page.php index 102c066ea0f..c61d0483576 100644 --- a/src/Structures/Page.php +++ b/src/Structures/Page.php @@ -162,7 +162,7 @@ public function uri() $uris = Blink::store('structure-uris'); - if ($cached = $uris[$this->reference] ?? null) { + if ($this->tree->uriCacheEnabled() && ($cached = $uris[$this->reference] ?? null)) { return $cached; } diff --git a/src/Structures/Tree.php b/src/Structures/Tree.php index 779f71ee9e2..6193f5fc896 100644 --- a/src/Structures/Tree.php +++ b/src/Structures/Tree.php @@ -19,6 +19,7 @@ abstract class Tree implements Contract, Localization protected $tree = []; protected $cachedFlattenedPages; protected $original; + protected $uriCacheEnabled = true; public function locale($locale = null) { @@ -117,6 +118,18 @@ public function uris() return $this->flattenedPages()->map->uri(); } + public function disableUriCache() + { + $this->uriCacheEnabled = false; + + return $this; + } + + public function uriCacheEnabled() + { + return $this->uriCacheEnabled; + } + public function page(string $id): ?Page { return $this->flattenedPages() diff --git a/src/UpdateScripts/AddUniqueSlugValidation.php b/src/UpdateScripts/AddUniqueSlugValidation.php new file mode 100644 index 00000000000..c8e32092c99 --- /dev/null +++ b/src/UpdateScripts/AddUniqueSlugValidation.php @@ -0,0 +1,54 @@ +isUpdatingTo('3.1.19'); + } + + public function update() + { + Collection::all()->each(function ($collection) { + // First, save them. This is the most reliable way at the moment + // to make sure the slug field exists in the blueprint properly. + $collection->entryBlueprints()->each->save(); + + Blink::forget('collection-entry-blueprints-'.$collection->handle()); + + $collection->entryBlueprints()->each(function ($blueprint) use ($collection) { + $this->updateBlueprint($collection, $blueprint); + }); + }); + + $this->console->warn("If you don't want unique slugs, remove the validation rule that was added to the slug fields."); + } + + private function updateBlueprint($collection, $blueprint) + { + $rules = $blueprint->field('slug')->get('validate'); + + // Make sure that it's an array, since it might + // be configured as a pipe delimited string. + $rules = Validator::explodeRules($rules); + + $rules[] = 'unique_entry_value:{collection},{id},{site}'; + $rules = array_unique($rules); + + $blueprint->ensureFieldHasConfig('slug', ['validate' => $rules]); + + $blueprint->save(); + + $this->console->line(sprintf( + 'Unique slug validation added to the %s collection\'s %s blueprint.', + $collection->handle(), + $blueprint->handle() + )); + } +} diff --git a/tests/Data/Entries/GetDateFromPathTest.php b/tests/Data/Entries/GetDateFromPathTest.php index 8a3c70f97ac..23cf4163b7c 100644 --- a/tests/Data/Entries/GetDateFromPathTest.php +++ b/tests/Data/Entries/GetDateFromPathTest.php @@ -7,11 +7,27 @@ class GetDateFromPathTest extends TestCase { - /** @test */ - public function it_gets_the_date_from_a_path() + /** + * @test + * @dataProvider paths + **/ + public function it_gets_the_date_from_a_path($expected, $path) { - $this->assertEquals('2015-01-01', (new GetDateFromPath)('path/to/2015-01-01.post.md')); - $this->assertEquals('2015-01-01-1300', (new GetDateFromPath)('path/to/2015-01-01-1300.post.md')); - $this->assertNull((new GetDateFromPath)('path/to/post.md')); + $this->assertEquals($expected, (new GetDateFromPath)($path)); + } + + public function paths() + { + return [ + 'date' => ['2015-01-01', 'path/to/2015-01-01.post.md'], + 'time' => ['2015-01-01-1300', 'path/to/2015-01-01-1300.post.md'], + 'no date' => [null, 'path/to/post.md'], + 'no date but slug with number' => [null, 'path/to/2nd-post.md'], + + 'date with id suffix' => ['2015-01-01', 'path/to/2015-01-01.post.id-suffix.md'], + 'time with id suffix' => ['2015-01-01-1300', 'path/to/2015-01-01-1300.post.id-suffix.md'], + 'no date with id suffix' => [null, 'path/to/post.id-suffix.md'], + 'no date but slug with number with id suffix' => [null, 'path/to/2nd-post.md'], + ]; } } diff --git a/tests/Data/Entries/GetSlugFromPathTest.php b/tests/Data/Entries/GetSlugFromPathTest.php new file mode 100644 index 00000000000..ece6b5e3d6c --- /dev/null +++ b/tests/Data/Entries/GetSlugFromPathTest.php @@ -0,0 +1,35 @@ +assertEquals($expected, (new GetSlugFromPath)('path/to/'.$path)); + } + + public function paths() + { + return [ + 'date' => ['post', '2015-01-01.post.md'], + 'time' => ['post', '2015-01-01-1300.post.md'], + 'no date' => ['post', 'post.md'], + 'no date but slug thats a number' => ['404', '404.md'], + 'no date but slug with number' => ['2nd-post', '2nd-post.md'], + + 'date with id suffix' => ['post', '2015-01-01.post.id-suffix.md'], + 'time with id suffix' => ['post', '2015-01-01-1300.post.id-suffix.md'], + 'no date with id suffix' => ['post', 'post.id-suffix.md'], + 'no date but slug thats a number' => ['404', '404.md'], + 'no date but slug with number with id suffix' => ['2nd-post', '2nd-post.md'], + ]; + } +} diff --git a/tests/Facades/UrlTest.php b/tests/Facades/UrlTest.php index aeb63038ec9..22147526b60 100644 --- a/tests/Facades/UrlTest.php +++ b/tests/Facades/UrlTest.php @@ -15,36 +15,6 @@ protected function resolveApplicationConfiguration($app) $app['config']->set('app.url', 'http://absolute-url-resolved-from-request.com'); } - public function testBuildsUrl() - { - $url = URL::buildFromPath('pages/about/index.md'); - $this->assertEquals('/about', $url); - } - - public function testBuildsHomepage() - { - $url = URL::buildFromPath('pages/index.md'); - $this->assertEquals('/', $url); - } - - public function testBuildsUrlFromFullPath() - { - $url = URL::buildFromPath(base_path().'/pages/index.md'); - $this->assertEquals('/', $url); - } - - public function testBuildsLocalizedUrl() - { - $url = URL::buildFromPath('pages/about/fr.index.md'); - $this->assertEquals('/about', $url); - } - - public function testBuildsLocalizedHomepage() - { - $url = URL::buildFromPath('pages/fr.index.md'); - $this->assertEquals('/', $url); - } - public function testPrependsSiteUrl() { Site::setConfig('sites.en.url', 'http://site.com/'); diff --git a/tests/Feature/Collections/UpdateCollectionStructureTest.php b/tests/Feature/Collections/UpdateCollectionStructureTest.php index cea4bfcf1fa..799037f9507 100644 --- a/tests/Feature/Collections/UpdateCollectionStructureTest.php +++ b/tests/Feature/Collections/UpdateCollectionStructureTest.php @@ -18,43 +18,97 @@ class UpdateCollectionStructureTest extends TestCase /** @test */ public function it_updates_the_tree() { + $this->withoutExceptionHandling(); $user = tap(User::make()->makeSuper())->save(); - $collection = tap(Collection::make('test'))->save(); - EntryFactory::id('1')->collection($collection)->create(); - EntryFactory::id('2')->collection($collection)->create(); - EntryFactory::id('3')->collection($collection)->create(); - EntryFactory::id('4')->collection($collection)->create(); - EntryFactory::id('5')->collection($collection)->create(); + $collection = tap(Collection::make('test')->routes('{parent_uri}/{slug}'))->save(); + EntryFactory::id('e1')->collection($collection)->slug('a')->create(); + EntryFactory::id('e2')->collection($collection)->slug('b')->create(); + EntryFactory::id('e3')->collection($collection)->slug('c')->create(); + EntryFactory::id('e4')->collection($collection)->slug('d')->create(); + EntryFactory::id('e5')->collection($collection)->slug('e')->create(); $collection->structureContents(['foo' => 'bar'])->save(); $collection->structure()->in('en')->tree([ - ['entry' => '1'], - ['entry' => '2'], - ['entry' => '3'], - ['entry' => '4'], - ['entry' => '5'], + ['entry' => 'e1'], + ['entry' => 'e2'], + ['entry' => 'e3'], + ['entry' => 'e4'], + ['entry' => 'e5'], ])->save(); $this ->actingAs($user) ->update($collection, ['pages' => [ - ['id' => '3', 'children' => [ - ['id' => '5', 'children' => [ - ['id' => '4', 'children' => []], + ['id' => 'e3', 'children' => [ + ['id' => 'e5', 'children' => [ + ['id' => 'e4', 'children' => []], ]], ]], - ['id' => '1', 'children' => []], - ['id' => '2', 'children' => []], + ['id' => 'e1', 'children' => []], + ['id' => 'e2', 'children' => []], ]]) ->assertOk(); $this->assertEquals([ - ['entry' => '3', 'children' => [ - ['entry' => '5', 'children' => [ - ['entry' => '4'], + ['entry' => 'e3', 'children' => [ + ['entry' => 'e5', 'children' => [ + ['entry' => 'e4'], ]], ]], - ['entry' => '1'], - ['entry' => '2'], + ['entry' => 'e1'], + ['entry' => 'e2'], + ], $collection->structure()->in('en')->tree()); + } + + /** @test */ + public function it_doesnt_update_the_tree_if_theres_a_duplicate_uri_when_expecting_root() + { + $this->duplicateUriTest(true); + } + + /** @test */ + public function it_doesnt_update_the_tree_if_theres_a_duplicate_uri_when_not_expecting_root() + { + $this->duplicateUriTest(false); + } + + private function duplicateUriTest($expectsRoot) + { + $user = tap(User::make()->makeSuper())->save(); + $collection = tap(Collection::make('test')->routes('{parent_uri}/{slug}'))->save(); + EntryFactory::id('e1')->collection($collection)->slug('alfa')->create(); + EntryFactory::id('e2')->collection($collection)->slug('bravo')->create(); + EntryFactory::id('e3')->collection($collection)->slug('bravo')->create(); + EntryFactory::id('e4')->collection($collection)->slug('charlie')->create(); + EntryFactory::id('e5')->collection($collection)->slug('delta')->create(); + $collection->structureContents(['foo' => 'bar', 'root' => $expectsRoot])->save(); + $tree = $collection->structure()->in('en'); + $tree->tree([ + ['entry' => 'e1'], + ['entry' => 'e3', 'children' => [ + ['entry' => 'e2'], + ]], + ['entry' => 'e4'], + ['entry' => 'e5'], + ])->save(); + + $this + ->actingAs($user) + ->update($collection, ['pages' => [ + ['id' => 'e1', 'children' => []], + ['id' => 'e2', 'children' => []], + ['id' => 'e3', 'children' => []], + ['id' => 'e4', 'children' => []], + ['id' => 'e5', 'children' => []], + ]]) + ->assertStatus(422); + + $this->assertEquals([ + ['entry' => 'e1'], + ['entry' => 'e3', 'children' => [ + ['entry' => 'e2'], + ]], + ['entry' => 'e4'], + ['entry' => 'e5'], ], $collection->structure()->in('en')->tree()); } @@ -63,20 +117,62 @@ public function it_deletes_entries_scheduled_for_deletion() { $user = tap(User::make()->makeSuper())->save(); $collection = tap(Collection::make('test'))->save(); - EntryFactory::id('1')->collection($collection)->create(); - EntryFactory::id('2')->collection($collection)->create(); - EntryFactory::id('3')->collection($collection)->create(); + EntryFactory::id('e1')->collection($collection)->create(); + EntryFactory::id('e2')->collection($collection)->create(); + EntryFactory::id('e3')->collection($collection)->create(); $collection->structureContents(['tree' => []])->save(); $this->assertCount(3, Entry::all()); $this ->actingAs($user) - ->update($collection, ['deletedEntries' => ['1', '3']]) + ->update($collection, ['deletedEntries' => ['e1', 'e3']]) ->assertOk(); $this->assertCount(1, Entry::all()); } + /** @test */ + public function it_doesnt_delete_entries_if_theres_a_duplicate_uri_validation_error() + { + $user = tap(User::make()->makeSuper())->save(); + $collection = tap(Collection::make('test')->routes('{parent_uri}/{slug}'))->save(); + EntryFactory::id('e1')->collection($collection)->create(); + EntryFactory::id('e2')->collection($collection)->create(); + EntryFactory::id('e3')->collection($collection)->create(); + EntryFactory::id('e4')->collection($collection)->create(); + $collection->structureContents(['root' => false])->save(); + $tree = $collection->structure()->in('en'); + $tree->tree([ + ['entry' => 'e1'], + ['entry' => 'e3', 'children' => [ + ['entry' => 'e2'], + ]], + ['entry' => 'e4'], + ])->save(); + $this->assertCount(4, Entry::all()); + + $this + ->actingAs($user) + ->update($collection, [ + 'deletedEntries' => ['e1'], + 'pages' => [ + ['id' => 'e1', 'children' => []], + ['id' => 'e2', 'children' => []], + ['id' => 'e3', 'children' => []], + ], + ]) + ->assertStatus(422); + + $this->assertCount(4, Entry::all()); + $this->assertEquals([ + ['entry' => 'e1'], + ['entry' => 'e3', 'children' => [ + ['entry' => 'e2'], + ]], + ['entry' => 'e4'], + ], $collection->structure()->in('en')->tree()); + } + /** @test */ public function it_denies_access_if_you_dont_have_permission_to_reorder() { diff --git a/tests/PathsTest.php b/tests/PathsTest.php index 916b36f05e8..e6fe450b050 100644 --- a/tests/PathsTest.php +++ b/tests/PathsTest.php @@ -6,21 +6,6 @@ class PathsTest extends TestCase { - public function testPathCleaning() - { - $path = Path::clean('/blog/2015-01-12.post.md'); - $this->assertEquals('/blog/post.md', $path); - - $path = Path::clean('pages/1.about/index.md'); - $this->assertEquals('pages/about/index.md', $path); - } - - public function testPathCleaningWithNumericSlug() - { - $path = Path::clean('/blog/404.md'); - $this->assertEquals('/blog/404.md', $path); - } - public function testRelativePath() { $path = Path::makeRelative(base_path().'/content/foo/bar.md'); diff --git a/tests/Stache/FeatureTest.php b/tests/Stache/FeatureTest.php index 4c763e7df91..823388d0752 100644 --- a/tests/Stache/FeatureTest.php +++ b/tests/Stache/FeatureTest.php @@ -65,7 +65,10 @@ public function it_gets_entry() $this->assertNull(Entry::find('users-john')); } - /** @test */ + /** + * @test + * @deprecated + **/ public function it_gets_entry_by_slug() { $this->assertEquals('Christmas', Entry::findBySlug('christmas', 'blog', 'christmas')->get('title')); diff --git a/tests/Stache/Repositories/EntryRepositoryTest.php b/tests/Stache/Repositories/EntryRepositoryTest.php index f3a9eae24ef..70b50943818 100644 --- a/tests/Stache/Repositories/EntryRepositoryTest.php +++ b/tests/Stache/Repositories/EntryRepositoryTest.php @@ -132,7 +132,10 @@ public function it_gets_entry_by_id() $this->assertNull($this->repo->find('unknown')); } - /** @test */ + /** + * @test + * @deprecated + **/ public function it_gets_entry_by_slug() { $entry = $this->repo->findBySlug('bravo', 'alphabetical'); diff --git a/tests/Stache/Stores/EntriesStoreTest.php b/tests/Stache/Stores/EntriesStoreTest.php index 8e3f3722f59..27088c1e7ef 100644 --- a/tests/Stache/Stores/EntriesStoreTest.php +++ b/tests/Stache/Stores/EntriesStoreTest.php @@ -58,9 +58,9 @@ public function it_gets_nested_files() $files = Traverser::filter([$store, 'getItemFilter'])->traverse($store); $this->assertEquals(collect([ - $dir.'/numeric/1.one.md', - $dir.'/numeric/2.two.md', - $dir.'/numeric/3.three.md', + $dir.'/numeric/one.md', + $dir.'/numeric/two.md', + $dir.'/numeric/three.md', ])->sort()->values()->all(), $files->keys()->sort()->values()->all()); }); @@ -130,6 +130,103 @@ public function it_saves_to_disk() $this->assertEquals($path, $this->parent->store('blog')->paths()->get('123')); } + /** @test */ + public function it_appends_suffix_to_the_filename_if_one_already_exists() + { + $existingPath = $this->directory.'/blog/2017-07-04.test.md'; + file_put_contents($existingPath, $existingContents = "---\nid: existing-id\n---"); + + $entry = Facades\Entry::make()->id('new-id')->slug('test')->date('2017-07-04')->collection('blog'); + $this->parent->store('blog')->save($entry); + $newPath = $this->directory.'/blog/2017-07-04.test.1.md'; + $this->assertFileEqualsString($existingPath, $existingContents); + $this->assertFileEqualsString($newPath, $entry->fileContents()); + + $anotherEntry = Facades\Entry::make()->id('another-new-id')->slug('test')->date('2017-07-04')->collection('blog'); + $this->parent->store('blog')->save($anotherEntry); + $anotherNewPath = $this->directory.'/blog/2017-07-04.test.2.md'; + $this->assertFileEqualsString($existingPath, $existingContents); + $this->assertFileEqualsString($anotherNewPath, $anotherEntry->fileContents()); + + $this->assertEquals($newPath, $this->parent->store('blog')->paths()->get('new-id')); + $this->assertEquals($anotherNewPath, $this->parent->store('blog')->paths()->get('another-new-id')); + + @unlink($newPath); + @unlink($anotherNewPath); + @unlink($existingPath); + $this->assertFileNotExists($newPath); + $this->assertFileNotExists($anotherNewPath); + $this->assertFileNotExists($existingPath); + } + + /** @test */ + public function it_doesnt_append_the_suffix_to_the_filename_if_it_is_itself() + { + $existingPath = $this->directory.'/blog/2017-07-04.test.md'; + file_put_contents($existingPath, "---\nid: the-id\n---"); + + $entry = Facades\Entry::make() + ->id('the-id') + ->slug('test') + ->date('2017-07-04') + ->collection('blog'); + + $this->parent->store('blog')->save($entry); + + $pathWithSuffix = $this->directory.'/blog/2017-07-04.test.1.md'; + $this->assertFileEqualsString($existingPath, $entry->fileContents()); + $this->assertEquals($existingPath, $this->parent->store('blog')->paths()->get('the-id')); + + @unlink($existingPath); + $this->assertFileNotExists($pathWithSuffix); + $this->assertFileNotExists($existingPath); + } + + /** @test */ + public function it_doesnt_append_the_suffix_to_an_already_suffixed_filename_if_it_is_itself() + { + $suffixlessExistingPath = $this->directory.'/blog/2017-07-04.test.md'; + file_put_contents($suffixlessExistingPath, "---\nid: the-id\n---"); + $suffixedExistingPath = $this->directory.'/blog/2017-07-04.test.md'; + file_put_contents($suffixedExistingPath, "---\nid: another-id\n---"); + + $entry = Facades\Entry::make() + ->id('another-id') + ->slug('test') + ->date('2017-07-04') + ->collection('blog'); + + $this->parent->store('blog')->save($entry); + + $pathWithIncrementedSuffix = $this->directory.'/blog/2017-07-04.test.2.md'; + $this->assertFileEqualsString($suffixedExistingPath, $entry->fileContents()); + @unlink($suffixedExistingPath); + $this->assertFileNotExists($pathWithIncrementedSuffix); + $this->assertFileNotExists($suffixedExistingPath); + + $this->assertEquals($suffixedExistingPath, $this->parent->store('blog')->paths()->get('another-id')); + } + + /** @test */ + public function it_keeps_the_suffix_even_if_the_suffixless_path_is_available() + { + $existingPath = $this->directory.'/blog/2017-07-04.test.1.md'; + $suffixlessPath = $this->directory.'/blog/2017-07-04.test.md'; + + file_put_contents($existingPath, 'id: 123'); + $entry = $this->parent->store('blog')->makeItemFromFile($existingPath, file_get_contents($existingPath)); + + $this->parent->store('blog')->save($entry); + + $this->assertFileEqualsString($existingPath, $entry->fileContents()); + $this->assertFileNotExists($suffixlessPath); + + $this->assertEquals($existingPath, $this->parent->store('blog')->paths()->get('123')); + + @unlink($existingPath); + $this->assertFileNotExists($existingPath); + } + /** @test */ public function it_ignores_entries_in_a_site_subdirectory_where_the_collection_doesnt_have_that_site_enabled() { diff --git a/tests/Stache/__fixtures__/content/collections/numeric/1.one.md b/tests/Stache/__fixtures__/content/collections/numeric/one.md similarity index 100% rename from tests/Stache/__fixtures__/content/collections/numeric/1.one.md rename to tests/Stache/__fixtures__/content/collections/numeric/one.md diff --git a/tests/Stache/__fixtures__/content/collections/numeric/3.three.md b/tests/Stache/__fixtures__/content/collections/numeric/three.md similarity index 100% rename from tests/Stache/__fixtures__/content/collections/numeric/3.three.md rename to tests/Stache/__fixtures__/content/collections/numeric/three.md diff --git a/tests/Stache/__fixtures__/content/collections/numeric/2.two.md b/tests/Stache/__fixtures__/content/collections/numeric/two.md similarity index 100% rename from tests/Stache/__fixtures__/content/collections/numeric/2.two.md rename to tests/Stache/__fixtures__/content/collections/numeric/two.md