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

Core Data: Clear auto-draft titles on save if not changed explicitly. #17633

Merged
merged 3 commits into from
Sep 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions packages/core-data/src/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -325,15 +325,16 @@ export function* saveEntityRecord(
yield receiveAutosaves( persistedRecord.id, updatedRecord );
}
} else {
// Auto drafts should be converted to drafts on explicit saves,
// Auto drafts should be converted to drafts on explicit saves and we should not respect their default title,
// but some plugins break with this behavior so we can't filter it on the server.
let data = record;
if (
kind === 'postType' &&
persistedRecord && persistedRecord.status === 'auto-draft' &&
! data.status
) {
data = { ...data, status: 'draft' };
if ( kind === 'postType' && persistedRecord && persistedRecord.status === 'auto-draft' ) {
if ( ! data.status ) {
data = { ...data, status: 'draft' };
}
if ( ! data.title || data.title === 'Auto Draft' ) {
Copy link
Member

Choose a reason for hiding this comment

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

Should Auto Draft be put through a translation function? It is translated in get_default_post_to_edit(). Not 100% sure if that is the same place the post title is getting set in Gutenberg.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How does that relate to this change? We are clearing the title here.

Copy link
Member

Choose a reason for hiding this comment

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

Well it compares the title with an English string, which might not work on a localized environment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

So is "Auto Draft" translated in this context? Where would we check?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, as that string is coming from get_default_post_to_edit().

So you'd need to compare it with __( 'Auto Draft' ), not just 'Auto Draft'

Copy link
Member

Choose a reason for hiding this comment

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

It looks like it is. I just tried @swissspidy's replication steps on trunk in a non-english locale, and the title was set to Automatisch gespeicherter Entwurf on publish.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to replicate it, but data.title is always undefined here unless you explicitly typed in the auto draft title, so the second check doesn't even run.

data = { ...data, title: '' };
}
}

// We perform an optimistic update here to clear all the edits that
Expand Down
25 changes: 25 additions & 0 deletions packages/e2e-tests/specs/change-detection.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,4 +323,29 @@ describe( 'Change detection', () => {

await assertIsDirty( false );
} );

it( 'should save posts without titles and persist and overwrite the auto draft title', async () => {
// Enter content.
await clickBlockAppender();
await page.keyboard.type( 'Paragraph' );

// Save
await Promise.all( [
// Wait for "Saved" to confirm save complete.
page.waitForSelector( '.editor-post-saved-state.is-saved' ),

// Keyboard shortcut Ctrl+S save.
pressKeyWithModifier( 'primary', 'S' ),
] );

// Verify that the title is empty.
const title = await page.$eval(
'.editor-post-title__input',
( element ) => element.innerHTML
);
expect( title ).toBe( '' );

// Verify that the post is not dirty.
await assertIsDirty( false );
} );
} );