-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Added e2e tests for tag.edited
webhook
#15555
Conversation
Codecov ReportBase: 52.32% // Head: 52.32% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #15555 +/- ##
=======================================
Coverage 52.32% 52.32%
=======================================
Files 1446 1446
Lines 93493 93493
Branches 10437 10437
=======================================
+ Hits 48920 48923 +3
+ Misses 43345 43344 -1
+ Partials 1228 1226 -2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Hey @dshubhadeep 👋🏻 Thanks for all the PRs 🙂 Can I ask that you include I fixed up your previous PR if you'd like to see how it's done: 426168f |
refs TryGhost#15537 - this adds an e2e test and test snapshot for the `tag.edited` webhook so we can prevent regressions and bugs in the future
f2370ae
to
95aefe8
Compare
@daniellockyer apologies. I've updated the message. |
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.
Minor refactor request, if it's possible. The rest is 👍
}) | ||
.matchBodySnapshot({ | ||
tag: { | ||
current: {...tagSnapshot, url: anyLocalURL}, |
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.
Imo, worth including the url: anyLocalURL
bit in the shared tagSnapshot
object now that it's been used in multiple places. Have you tried moving it @dshubhadeep ?
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.
Yes, it was done like that before. But tests were failing for tag.deleted webhook since the previous
key in the snapshot didn't have the URL.
@naz Maybe I could create a new variable something like snapshotWithUrl ?
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.
Lets keep the extraction to the rule of 3. If we have to reuse the same form 3rd time lets extract a tagSnapshotWithURL
like you proposed or create a builder function having a "withURL: boolean" parameter. It's been used 2 times now, so it's good to go as is 👍
tag.edited
webhooktag.edited
webhook
Hi @dshubhadeep thanks so much for this PR. Sorry for the delay, but this has now been merged 🎉 and will appear in the next release of Ghost - usually Fridays. I'm not sure if you found this through hacktoberfest, but I've added the accepted label to this PR to make sure it counts. We have plenty of open issues and ongoing projects for hacktoberfest and beyond. We would love to see you around again 👋 |
refs: #15537 - this adds an e2e test and test snapshot for the `tag.edited` webhook so we can prevent regressions and bugs in the future Co-authored-by: Hannah Wolfe <[email protected]>
Added e2e tests for tag.edited webhook
Ref - #15537