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

Purchases: Add cancel/remove option for expired domain registrations and mappings #470

Merged
merged 8 commits into from
Nov 27, 2015

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Nov 23, 2015

Fixes #241.

This PR adds remove information on Manage Purchase page for purchases that are expired and can be cancelled.

Expected view:
screen shot 2015-11-26 at 09 08 58

Testing

  1. Go to http://calypso.localhost:3000/purchases.
  2. Selected expired domain or domain mapping purchase.
  3. You should see remove information presented above.

@gziolo gziolo added [Status] In Progress [Type] Task [Feature] Purchase Management Related to managing purchases such as subscriptions, plans, history, auto-renew, cancellation, etc. labels Nov 23, 2015
@gziolo gziolo self-assigned this Nov 23, 2015
@gziolo gziolo added this to the Purchases: v1 milestone Nov 23, 2015
@gziolo gziolo force-pushed the add/cancel-expired-purchases branch 3 times, most recently from 052a2af to 342fdea Compare November 23, 2015 10:13
@gziolo gziolo added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] In Progress and removed [Status] In Progress [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 23, 2015
@gziolo
Copy link
Member Author

gziolo commented Nov 24, 2015

@mikeshelton1503: even when we fixed domain Cancel and Refund flow, we still se the same error message for expired domains. Let's display user message with prompt to contact support for now, and fix it in v2, what do you think? Can you provide full message text?

@mikeshelton1503
Copy link
Contributor

Sounds good to me. Here's the implementation for the message:

image

Looking to cancel? Please contact support to cancel [yourdomainname.com].

@fabianapsimoes
Copy link
Contributor

Is it really "cancel" in this case? Or should we stick to "remove" as planned for the full feature?

@mikeshelton1503
Copy link
Contributor

Yes, sorry you're right, it should be:

Looking to remove this domain? Please contact support to remove [yourdomainname.com] from your account.

@gziolo gziolo added the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Nov 26, 2015
@gziolo
Copy link
Member Author

gziolo commented Nov 26, 2015

@mikeshelton1503 I replace the word domain with purchase in the message displayed to the user. We need to keep in mind that this message can be displayed also for domain mapping. There is opacity filter applied on expired purchase's page:

screen shot 2015-11-26 at 09 08 58

I'm waiting for approval from your side :)

@gziolo gziolo force-pushed the add/cancel-expired-purchases branch from 36d8308 to aa9592d Compare November 26, 2015 08:14
@gziolo gziolo added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Nov 26, 2015
@scruffian
Copy link
Member

I think we can merge this without mike's sign off given he's AFK. When he's back he can open an issue if he wants changes.

@gziolo
Copy link
Member Author

gziolo commented Nov 26, 2015

Yes, let's do that this way.

@gziolo gziolo removed the [Status] Needs Design Review Add this when you'd like to get a review / feedback from the Design team on your PR label Nov 26, 2015
return false;
}

return ( isExpired( purchase ) && purchase.isCancelable );
Copy link
Member

Choose a reason for hiding this comment

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

Why are we accessing the isCancelable property here rather than using the isCancelable function? I think that creates some confusion. I'm confused anyway!

Copy link
Member Author

Choose a reason for hiding this comment

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

We display only message at the moment, I think we can remove purchase.isCancelable check. I wanted to keep it similar to /my-upgrades, but as we can't cancel manually expired purchases we need to wait with this until v2.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed method definition to make it clearer. Commiting soon 👍

@scruffian scruffian added [Status] Needs Author Reply and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 26, 2015
@@ -431,6 +432,32 @@ const ManagePurchase = React.createClass( {
}
},

renderRemovePurchaseInformation() {
const purchase = getPurchase( this.props ),
contactSupportUrl = 'https://support.wordpress.com/';
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

There is really no standard way, there are urls with locale and without. Both work properly. When there is no locale provided it redirects to your lang version, so in long run this seems to be better approach to let server decide which locale load.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch 👍

@scruffian
Copy link
Member

Nice addition of 949f522.

LGTM 👍

@scruffian scruffian added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Nov 27, 2015
@gziolo
Copy link
Member Author

gziolo commented Nov 27, 2015

Thanks for help on that one @scruffian!

gziolo added a commit that referenced this pull request Nov 27, 2015
Purchases: Add cancel/remove option for expired domain registrations and mappings
@gziolo gziolo merged commit e08afcc into master Nov 27, 2015
@gziolo gziolo deleted the add/cancel-expired-purchases branch November 27, 2015 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Purchase Management Related to managing purchases such as subscriptions, plans, history, auto-renew, cancellation, etc. [Type] Task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants