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

Podcast Player: Show error from API #17537

Merged
merged 2 commits into from Oct 22, 2020
Merged

Podcast Player: Show error from API #17537

merged 2 commits into from Oct 22, 2020

Conversation

marekhrabe
Copy link
Contributor

Fixes #17496

Changes proposed in this Pull Request:

  • Surface errors from the Podcast Player API - they contain useful context of the exact error.
  • Only use the generic error as a fallback

Jetpack product discussion

Does this pull request change what data or activity we track or use?

no

Testing instructions:

  • Test for example with an empty feed
  • or force the API error like I did:
    • at the top of the method get_player_data in _inc/lib/class-jetpack-podcast-helper.php,
    • return new WP_Error( 'err_code', __( 'Custom error message', 'jetpack' ) );

Proposed changelog entry for your changes:

  • More helpful error messages in the Podcast Player block

@marekhrabe marekhrabe added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Block] Podcast Player labels Oct 20, 2020
@marekhrabe marekhrabe self-assigned this Oct 20, 2020
@matticbot
Copy link
Contributor

Caution: This PR has changes that must be merged to WordPress.com
Hello marekhrabe! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D51472-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

@jetpackbot
Copy link

Warnings
⚠️

The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-17537

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against d880cea

@marekhrabe marekhrabe added [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Status] Needs Team Review labels Oct 20, 2020
Copy link
Contributor

@cpapazoglou cpapazoglou left a comment

Choose a reason for hiding this comment

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

Tested with an empty podcast https://anchor.fm/s/3b4e47ac/podcast/rss and it surfaced the error correctly

extensions/blocks/podcast-player/edit.js Show resolved Hide resolved
@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. [Status] Needs Team Review labels Oct 21, 2020
@jeherve jeherve added this to the 9.1 milestone Oct 21, 2020
@jeherve jeherve merged commit 9b36316 into master Oct 22, 2020
@jeherve jeherve deleted the add/podcast-player-errors branch October 22, 2020 09:48
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Ready to Merge Go ahead, you can push that green button! labels Oct 22, 2020
jeherve added a commit that referenced this pull request Oct 27, 2020
@obenland
Copy link
Member

r218369-wpcom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Podcast Player Touches WP.com Files [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Podcast Player: Surface error message from API
6 participants