wp-calypso icon indicating copy to clipboard operation
wp-calypso copied to clipboard

Published 20 hours ago • verified publisher icon Automattic

Remove environment url conversion from invite__revoke.ts

Open youbrokesomething opened this issue 2 years ago • 4 comments

Proposed Changes

  • e2e/specs/users/invite__revoke.ts started failing due to a change in the expected invite URL. Starting sometime Sept 19th, the invite URL no longer has environment-specific base URLs and does not need to be adjusted. This change uses the original URL for a goto but leaves the data-helper function in case we need to adjust URLs for specific environments again in the future.

Testing Instructions

  • invite__revoke.ts should pass local runs.
  • Group calypso-release should pass.

Pre-merge Checklist

  • [x] Have you written new tests for your changes?
  • [ ] Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • [ ] Have you checked for TypeScript, React or other console errors?
  • [x] Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • [x] Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?

Related to #

youbrokesomething avatar Sep 20 '22 15:09 youbrokesomething

Calypso Live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-45017
Jetpack Cloud live (direct link)
https://calypso.live?image=registry.a8c.com/calypso/app:build-45017&env=jetpack

github-actions[bot] avatar Sep 20 '22 15:09 github-actions[bot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

matticbot avatar Sep 20 '22 15:09 matticbot

Starting sometime Sept 19th, the invite URL no longer has environment-specific base URLs and does not need to be adjusted

Interesting. I wonder which backend team made this change and why - it isn't a bad change per se, but it would have been nice to get a heads up on this and coordinate the change.

In any case, thanks for working on it!

worldomonation avatar Sep 20 '22 17:09 worldomonation

@youbrokesomething I ran some manual checks and while the URL structure is different, I think we can still make this work - just might need to do some additional parsing of the URL. I'll see if there's a non-hacky way to do that.

worldomonation avatar Sep 22 '22 16:09 worldomonation

@youbrokesomething I'm catching up on open PRs that I might be able to help with! This looks GTG to me, anything else you want eyes on before 🚢 ?

dpasque avatar Oct 06 '22 18:10 dpasque