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

LiveRadio Refactor Regarding Ares Response #6207

Merged
merged 15 commits into from
Apr 17, 2020
Merged

Conversation

rebeccamcginn
Copy link
Contributor

@rebeccamcginn rebeccamcginn commented Apr 16, 2020

Preparation for #6134

Overall change:
Refactoring to only pass the required values to page components instead of the full Ares response

Code changes:

  • Stripped down the Ares response to pass along only necessary values
  • Refactored LiveRadio ATI Analytics to handle new data format
  • Refactored Chartbeat to handle new data format
  • Renamed RadioPage to LiveRadioPage

  • I have assigned myself to this PR and the corresponding issues
  • I have added labels to this PR for the relevant pod(s) affected by these changes
  • I have assigned this PR to the Simorgh project

Testing:

  • Automated (jest and/or cypress) tests added (for new features) or updated (for existing features)
  • If necessary, I have run the local E2E non-smoke tests relevant to my changes (CYPRESS_APP_ENV=local CYPRESS_SMOKE=false npm run test:e2e:interactive)
  • This PR requires manual testing

@rebeccamcginn rebeccamcginn self-assigned this Apr 16, 2020
@rebeccamcginn rebeccamcginn added the ws-media World Service Media label Apr 16, 2020
@@ -1395,7 +1395,7 @@ exports[`Given I am on the Korean Live Radio page When I view the source code in
</footer>
</div>
<script>
window.SIMORGH_DATA={"status":200,"pageData":{"content":{"blocks":[{"text":"BBC 코리아 라디오","markupType":"plain_text","type":"heading","uuid":"mock-uuid"},{"text":"세계와 한반도 뉴스를 공정하고 객관적으로 전달해 드립니다","type":"paragraph","uuid":"mock-uuid"},{"id":"mock-id","subType":"primary","format":"audio","externalId":"bbc_korean_radio","duration":"PT0S","caption":"","embedding":false,"available":true,"live":true,"type":"version","uuid":"mock-uuid"}]},"metadata":{"id":"mock-id","locators":{"pipsUrn":"urn:bbc:pips:bbc_korean_radio"},"type":"WS-LIVE","createdBy":"korean","language":"ko","lastUpdated":0,"firstPublished":0,"lastPublished":0,"options":{},"analyticsLabels":{"pageTitle":"BBC 코리아 라디오 - BBC News 코리아","pageIdentifier":"korean.bbc_korean_radio.liveradio.page","producerId":"57","contentType":"player-live","producer":"KOREAN"},"tags":{},"version":"1.1.2","blockTypes":["heading","version","paragraph"]},"promo":{"name":"BBC 코리아 라디오","summary":"세계와 한반도 뉴스를 공정하고 객관적으로 전달해 드립니다","uri":"/korean/bbc_korean_radio/liveradio","contentType":"WS-LIVE","id":"mock-id","type":"link"},"relatedContent":{"site":{"subType":"simple","name":"korean","uri":"/korean","type":"simple"},"groups":[]}},"path":"/korean/bbc_korean_radio/liveradio","timeOnServer": "mock-time"}
window.SIMORGH_DATA={"status":200,"pageData":{"content":{"blocks":[{"text":"BBC 코리아 라디오","markupType":"plain_text","type":"heading","uuid":"mock-uuid"},{"text":"세계와 한반도 뉴스를 공정하고 객관적으로 전달해 드립니다","type":"paragraph","uuid":"mock-uuid"},{"id":"mock-id","subType":"primary","format":"audio","externalId":"bbc_korean_radio","duration":"PT0S","caption":"","embedding":false,"available":true,"live":true,"type":"version","uuid":"mock-uuid"}]},"language":"ko","id":"mock-id","name":"BBC 코리아 라디오","summary":"세계와 한반도 뉴스를 공정하고 객관적으로 전달해 드립니다","pageTitle":"BBC 코리아 라디오 - BBC News 코리아","contentType":"player-live","pageIdentifier":"korean.bbc_korean_radio.liveradio.page"},"path":"/korean/bbc_korean_radio/liveradio","timeOnServer": "mock-time"}

Choose a reason for hiding this comment

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

oh wow. didn't realise this strategy would also cut down on the size of this object that hydrates the page on the client. For some pages it's absolutely huge. this is cool 😄

@rebeccamcginn
Copy link
Contributor Author

Chartbeat requires further testing on AMP

Copy link

@jroebu14 jroebu14 left a comment

Choose a reason for hiding this comment

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

looking good!

@rebeccamcginn rebeccamcginn marked this pull request as ready for review April 17, 2020 12:57
@karinathomasbbc
Copy link
Contributor

Nice 👍

@karinathomasbbc
Copy link
Contributor

karinathomasbbc commented Apr 17, 2020

Oh, one thought I just had - will this log statement still output WS-LIVE ->

pageType: pathOr('Error', ['pageData', 'metadata', 'type'], data),
- we use this in SumoLogic.

I expect the see the following output in the logs:

2020-04-17 14:07:45.257 info [server/index.jsx] {
  "event": "routing_info",
  "message": {
    "url": "/uzbek/bbc_uzbek_radio/liveradio",
    "status": 200,
    "pageType": "WS-LIVE"
  }
}

@rebeccamcginn
Copy link
Contributor Author

Oh, one thought I just had - will this log statement still output WS-LIVE ->

pageType: pathOr('Error', ['pageData', 'metadata', 'type'], data),

  • we use this in SumoLogic

No, it won't get the pageType for liveradio or onDemand radio

@karinathomasbbc
Copy link
Contributor

No, it won't get the pageType for liveradio or onDemand radio

😢

@rebeccamcginn rebeccamcginn merged commit e3d1bd9 into latest Apr 17, 2020
@rebeccamcginn rebeccamcginn deleted the onDemandATIWork branch April 17, 2020 15:29
@karinathomasbbc karinathomasbbc linked an issue Apr 17, 2020 that may be closed by this pull request
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ws-media World Service Media
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On Demand Radio Page has correct ATI page metadata
4 participants