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

FLAG-866: fetch notifications and pass them as props to Header #4686

Merged
merged 1 commit into from
Sep 8, 2023

Conversation

wri7tno
Copy link
Collaborator

@wri7tno wri7tno commented Sep 6, 2023

Fetching notifications for all pages. Notifications aren't visible yet, that is going to be addressed in a separate ticket.

Overview

Brief description of what this PR does, and why it is needed.

Demo

If applicable: screenshots, gifs, etc.

Notes

If applicable: ancilary topics, caveats, alternative strategies that didn't work out, etc.

Testing

  • Include any steps to verify the features this PR introduces.
  • If this fixes a bug, include bug reproduction steps.

@wri7tno wri7tno self-assigned this Sep 6, 2023
@willian-viana willian-viana temporarily deployed to gfw-staging-pr-4686 September 6, 2023 03:50 Inactive
@wri7tno wri7tno force-pushed the feat/FLAG-866--consume-notifications branch from 1f53d6f to 2df9dc0 Compare September 6, 2023 15:52
@wri7tno wri7tno temporarily deployed to gfw-staging-pr-4686 September 6, 2023 15:53 Inactive
@wri7tno wri7tno force-pushed the feat/FLAG-866--consume-notifications branch from 2df9dc0 to 5ff1b15 Compare September 7, 2023 17:25
@wri7tno wri7tno marked this pull request as ready for review September 7, 2023 17:33
Copy link
Collaborator

@willian-viana willian-viana left a comment

Choose a reason for hiding this comment

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

:shipit:

// eslint-disable-next-line no-undef
jest.mock('axios');

describe('getPublishedNotifications', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

AWESOME! 🚀

},
];

axios.get.mockResolvedValueOnce(response);
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happened when the result is a empty array? Do you think we need to create a test for this case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not in this case since the empty array doesn't change anything, we are displaying a collection of items.

@wri7tno wri7tno force-pushed the feat/FLAG-866--consume-notifications branch from 5ff1b15 to ab47872 Compare September 8, 2023 15:44
@wri7tno wri7tno temporarily deployed to gfw-staging-pr-4686 September 8, 2023 15:44 Inactive
@wri7tno wri7tno merged commit ac28af5 into develop Sep 8, 2023
4 checks passed
@wri7tno wri7tno deleted the feat/FLAG-866--consume-notifications branch September 8, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants