Pro 5937 move peer page #4530
Pro 5937 move peer page #4530
Conversation
modules/@apostrophecms/page/index.js
Outdated
| slugStem += '/'; | ||
| } | ||
| moved.slug = moved.slug.replace(matchOldParentSlugPrefix, self.apos.util.addSlashIfNeeded(parent.slug)); | ||
| const relativeSlug = path.relative(parent.slug, moved.slug); |
There was a problem hiding this comment.
Not suitable for page slugs because on some platforms it may return a platform-dependent path separator.
There was a problem hiding this comment.
I know right now our documentation says native Windows is not supported, but it's not best practice to use path contrary to its intended purpose, and the way things are going with feedback from customers I think we will have to ease up and support native Node.js on Windows soon so let's not dig the hole deeper on that
test/pages.js
Outdated
| @@ -457,6 +457,79 @@ describe('Pages', function() { | |||
| assert.strictEqual(page.path, `${homeId.replace(':en:published', '')}/another-parent/parent/sibling`); | |||
| }); | |||
|
|
|||
| it('moving peer /parent/peer-page into /parent should ends with /parent/peer-page', async function() { | |||
There was a problem hiding this comment.
Please add coverage of the more common case as well, e.g. where the peers have slugs /a and /b and the outcome should be /a/b. Looking quickly over the existing tests I am not convinced we have that covered right now.
* main: cleanup unset marks on heading return language release 4.3.0 fixes changelog typo uses NODE_ENV instead of APOS_DEV to guess vue alias simplifies vueProjectLevelPath const by using path.resolve prioritizes project level vue installation adds changelog adds function to set vue alias unique instance in dev mode Add alternative fix for "ungrouped" tab Hide single tab fix image cropper stencil getting smaller when changing aspect ratio (#4529) make a sparate AposWidgetModalTabs used only by AposWidgetEditor pass a limit prop to AposModalTabs fix cypress tests
| .slice(0, -1) | ||
| .join('/'); | ||
|
|
||
| moved.slug = parent.slug.endsWith(movedSlugCandidate) |
There was a problem hiding this comment.
Also I think this could be confused in the following scenario:
These are peers:
/foo
/foobar
(Note no second slash)
And the second one gets moved under the first.
That should result in:
/foo/foobar
(Because no slash)
Tests should cover this case.
There was a problem hiding this comment.
This is in addition to the concern about testing plain old:
/foo
/bar
To make sure that when /bar is moved under /foo we still get /foo/bar in that typical case.
* main: Add test attributes release 4.3.2: hotfix for widget validation Fix widget validation, improve tabs annotation and UX fix migration bug that impacted our client demo, could impact other projects with stray documents corresponding to no module remove log when pasting content into RT dont force fit a valid class Center widget tab text
| @@ -2,6 +2,35 @@ | |||
|
|
|||
| ## UNRELEASED | |||
|
|
|||
| ### Changes | |||
|
|
|||
| * Improves widget tabs for the hidden entries, improves UX when validation errors are present in non-focused tabs. | |||
CHANGELOG.md
Outdated
|
|
||
| * Databases containing documents that no longer correspond to any module no longer cause the migration that adds missing mode properties | ||
| to fail (an issue introduced in version 4.2.0). Databases with no such "orphaned" documents were not affected. | ||
| >>>>>>> main |
* main: Pro 5937 move peer page (#4530)
Summary
Recognize when the slug of a new child already contains the new parent's slug and not double it
What are the specific steps to test this change?
npx mocha test/pages.jsmust be green.What kind of change does this PR introduce?
(Check at least one)
Make sure the PR fulfills these requirements:
If adding a new feature without an already open issue, it's best to open a feature request issue first and wait for approval before working on it.
Other information: