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

Prevent Homepage from being deleted or moved to a draft #14061

Merged
merged 7 commits into from
Feb 16, 2021

Conversation

chipsnyder
Copy link
Contributor

@chipsnyder chipsnyder commented Feb 12, 2021

Fixes: #13908

To test:

📓 Make sure your site is set to have a Static Homepage

Attempt to delete Homepage

  1. Navigate to the Page List
  2. Select the ... on the Homepage
  3. Expect to NOT see the Move to Trash option
    📓 If the Homepage has been moved out of the Published filter then the Move to Trash option will still be there. The site is already broken so no reason to restrict it.
Screenshot

Attempt to move Homepage to Draft - Page List

📓 This doesn't apply if you've trashed your Homepage.

  1. Navigate to the Page List
  2. Select the ... on the Homepage
  3. Expect to NOT see the Move to Draft
Screenshot

Attempt to move Homepage to Draft - Page Settings

  1. Navigate to the Page List
  2. Select Homepage to open the Editor
  3. Select the ... in the top right-hand corner
  4. Select Page Settings
  5. Select Status
  6. Expect to only have the Publish option and Private Option.
    📓 On iOS Private is handled by a different menu.
Screenshot

Attempt to delete Page

  1. Navigate to the Page List
  2. Select the ... on a page that is not the Homepage
  3. Expect to see the Move to Trash option
  4. Select Move to Trash
  5. Expect for the page to be deleted

Attempt to move Page to Draft - Page List

  1. Navigate to the Page List
  2. Select the ... on a page that is not the Homepage
  3. Select Move to Draft
  4. Expect to see the page be moved to a draft.

Attempt to move Page to Draft - Page Settings

  1. Navigate to the Page List
  2. Select a page (not Homepage) to open the Editor
  3. Select the ... in the top right-hand corner
  4. Select Page Settings
  5. Select Status
  6. Change the status
  7. Expect to see the screen to dismiss and the page will modify to the new status.

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 12, 2021

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@chipsnyder chipsnyder linked an issue Feb 12, 2021 that may be closed by this pull request
@chipsnyder chipsnyder marked this pull request as ready for review February 12, 2021 20:44
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Feb 12, 2021

You can test the changes on this Pull Request by downloading the APK here.

Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Nice work @chipsnyder 👍
Tested the App on an Pixel 2 XL with Android 11 and behaves as expected.
The code changes are consistent and LGTM 🎉

Regarding the failing checks:

  1. The CreatePageListItemActionsUseCaseTest tests should be ok by removing the MOVE_TO_DRAFT, MOVE_TO_TRASH values from the expected actions. It would be nice if we added a couple more tests to verify the new if branch in the CreatePageListItemActionsUseCase.

  2. The lint check failure has to do with the introduction of Detekt and a fix is already available in develop

@chipsnyder
Copy link
Contributor Author

Regarding the failing checks:

  1. The CreatePageListItemActionsUseCaseTest tests should be ok by removing the MOVE_TO_DRAFT, MOVE_TO_TRASH values from the expected actions. It would be nice if we added a couple more tests to verify the new if branch in the CreatePageListItemActionsUseCase.

  2. The lint check failure has to do with the introduction of Detekt and a fix is already available in develop

Thanks for the help @antonis! I totally forgot to come back and validate these checks before I logged off for the day. I updated the tests that check the available settings for Homepages and it looks like a test already existed for pages that aren't homepages.

@chipsnyder
Copy link
Contributor Author

The installable build is still failing but that should be resolved by #14082 once that's available

Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Thanks for the help @antonis! I totally forgot to come back and validate these checks before I logged off for the day. I updated the tests that check the available settings for Homepages and it looks like a test already existed for pages that aren't homepages.

Thank you for the changes @chipsnyder 👍
LGTM 🎉

@chipsnyder chipsnyder merged commit 9f9a9c9 into develop Feb 16, 2021
@chipsnyder chipsnyder deleted the issue/13908-homepage branch February 16, 2021 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User can delete Homepage and break their site
2 participants