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

Checklist: Update task url based on headstarted posts #26758

Merged
merged 1 commit into from
Aug 20, 2018

Conversation

taggon
Copy link
Contributor

@taggon taggon commented Aug 17, 2018

A checklist task may need to change the destination URL based on the posts. For instance, "Publish your first blog post" task create a new post by default. However, once a user published a post, the task should head for it instead.

This url logic is currently placed on the server-side, but it would be better if we could move the logic to Calypso. That's what this PR handles.

How to test

  1. Create a new site with default settings.
  2. Apply D17317-code to your sandbox that points public-api.wordpress.com
  3. Empty and re-headstart the site you just created by following the instruction on 1efe0-pb
    4-1. "Personalize your Contact page" task should head for the new contact page instead of /post/YOUR_SITE/2.
    4-2. "Publish your first blog post" task should not create a new post.

@taggon taggon added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Checklist labels Aug 17, 2018
@taggon taggon self-assigned this Aug 17, 2018
@matticbot
Copy link
Contributor

@taggon taggon requested a review from sirreal August 17, 2018 12:56
@taggon taggon force-pushed the checklist/update-task-url-based-on-headstarted-posts branch from 791e2d7 to d987968 Compare August 17, 2018 13:45
@taggon
Copy link
Contributor Author

taggon commented Aug 17, 2018

Rebased.

@sirreal sirreal requested a review from ockham August 17, 2018 15:21
@dzver
Copy link
Member

dzver commented Aug 20, 2018

It looks good and works fine. Will the PR also work well if the site has, let's say, 1000 posts?

EDIT: of course it will, I noticed that the order is by ID ASC only after creating a test site with 10 test pages :)

@dzver dzver self-requested a review August 20, 2018 06:50
Copy link
Member

@dzver dzver left a comment

Choose a reason for hiding this comment

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

LGTM

@taggon
Copy link
Contributor Author

taggon commented Aug 20, 2018

@dzver Yes, it will. With this update, the checklist retrieves first 10 posts only(including pages). I don't think it is too big to handle. :)
Thanks! 🎉

@taggon taggon merged commit 83d7050 into master Aug 20, 2018
@taggon taggon deleted the checklist/update-task-url-based-on-headstarted-posts branch August 20, 2018 12:46
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 20, 2018
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.

3 participants