-
Notifications
You must be signed in to change notification settings - Fork 1
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
MHP-3132-Render-Reported-Stories-and-Comments #1443
Conversation
…s, update tests and snapshots
@OzzieOrca This is my first pass at this but more changes will be coming as Robert's pr #1444 is touching some of the same screens. We want to get his ticket out first, but I figured you could take a look and give any suggestions on how I can improve what I have here 🙂 |
…ct-native into MHP-3132-Render-Reported-Stories-and-Comment | Fix Merge Conflicts
@OzzieOrca This should be good to review. Hopefully the commit where I fixed the conflicts with me and Robert's changes is not too messy. |
const subject = { | ||
id: 'commentId', | ||
__typename: 'CommunityCelebrationItemComment' as 'CommunityCelebrationItemComment', | ||
content: 'something', | ||
createdAt: '2020-01-15T20:58:04Z', | ||
typeName: 'CommunityCelebrationItemComment', | ||
updatedAt: '2020-01-15T20:58:04Z', | ||
id: 'commentId', | ||
person: { | ||
__typename: 'Person' as 'Person', | ||
fullName: 'Christian Huffman', | ||
firstName: 'Christian', | ||
lastName: 'Huffman', | ||
id: 'someid', | ||
}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should eventually use fragments instead of manually mocking all of this... See #1444 (comment). I don't want to hold up your PR though...
@@ -124,13 +128,16 @@ describe('report item', () => { | |||
const StoryProps = { | |||
...props, | |||
item: { | |||
__typename: 'ContentComplaint' as 'ContentComplaint', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So for some reason typescript yelled at me for just having 'ContentComplaint'. So I had to type ContentComplaint as ContentComplaint(?). I was a little confused because both are just strings, but I guess it wants a specific string? Had to do this is a couple of other places for the same reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya I encountered that somewhere too. It's typed really specifically. This is fine until we can automock it.
Converted GroupReport to use graphql, update other components to hooks, update tests and snapshots