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

update faq and faq_de regarding in-app statistics #3151

Merged
merged 4 commits into from
Oct 19, 2022
Merged

Conversation

brianebeling
Copy link
Member

@brianebeling brianebeling commented Oct 19, 2022

Fixes #1497

The red lines roughly highlight the area of changes

DE:
image

EN:
image


Internal Tracking ID: EXPOSUREAPP-8383

@dsarkar dsarkar marked this pull request as ready for review October 19, 2022 10:01
@dsarkar dsarkar requested review from a team and GisoSchroederSAP October 19, 2022 10:01
@Ein-Tim
Copy link
Contributor

Ein-Tim commented Oct 19, 2022

This fixes: #1497

@MikeMcC399
Copy link
Contributor

This PR breaks the Cypress test https://github.com/corona-warn-app/cwa-website/blob/master/cypress/integration/faq.js

Copy link
Contributor

@MikeMcC399 MikeMcC399 left a comment

Choose a reason for hiding this comment

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

Do not merge until Cypress issues have been fixed.

Unfortunately the faq.js test relies on exact text in this FAQ (#further_details). This is another example of a questionable test strategy, but in the short term the solution is to change the text in https://github.com/corona-warn-app/cwa-website/tree/master/cypress/fixtures/copy

Also, the guidance in https://github.com/corona-warn-app/cwa-website/blob/master/README.md#testing says:

"To minimize the occurrence of errors we would ask you to perform all tests when contributing to our repository."

This step was omitted apparently.

@dsarkar dsarkar marked this pull request as draft October 19, 2022 11:08
@MikeMcC399
Copy link
Contributor

@brianebeling
Copy link
Member Author

Thank you for highlighting the problem and providing a solution. I will review your PR shortly.

Copy link
Contributor

@GisoSchroederSAP GisoSchroederSAP left a comment

Choose a reason for hiding this comment

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

Great, one update of the article covers several updates of the app, thanks a million!
Approved.

@brianebeling
Copy link
Member Author

Since PR #3153 has been merged, this should no longer break the tests, as tested by @MikeMcC399 and me.

@brianebeling brianebeling marked this pull request as ready for review October 19, 2022 14:18
@dsarkar
Copy link
Member

dsarkar commented Oct 19, 2022

@MikeMcC399 Thanks for pointing out the cypress issue and reviewing.

@dsarkar dsarkar merged commit 0677dab into master Oct 19, 2022
@dsarkar dsarkar deleted the in-app-statistics branch October 19, 2022 14:22
@larswmh larswmh mentioned this pull request Nov 15, 2022
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants