Skip to content

feat: Manually capturing user feedback - #559

Merged
bruno-garcia merged 5 commits into
getsentry:mainfrom
lucas-zimerman:feature/manualUserReport
Oct 29, 2020
Merged

feat: Manually capturing user feedback#559
bruno-garcia merged 5 commits into
getsentry:mainfrom
lucas-zimerman:feature/manualUserReport

Conversation

@lucas-zimerman

Copy link
Copy Markdown
Collaborator

Based on getsentry/sentry-cocoa#805

This PR implements the CaptureUserFeedback so users can associate user feedback to an event.

Fixes#153

@lucas-zimerman

Copy link
Copy Markdown
CollaboratorAuthor

🤔 Should I edit the changelog?

@bruno-garciabruno-garcia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for contributing again Lucas! I left a few comments.

Comment threadsamples/Sentry.Samples.Console.ManualUserFeedback/Program.cs Outdated
Comment threadsamples/Sentry.Samples.Console.ManualUserFeedback/Program.cs Outdated
Comment threadsamples/Sentry.Samples.Console.ManualUserFeedback/Program.cs Outdated
Comment threadsrc/Sentry/ISentryClient.cs Outdated
Comment threadsrc/Sentry/Protocol/SentryUserFeedback.cs Outdated
Comment threadsrc/Sentry/SentryClient.cs Outdated
Comment threadsrc/Sentry/SentrySdk.cs Outdated
Comment threadtest/Sentry.Tests/SentryClientTests.cs Outdated
Comment threadtest/Sentry.Tests/SentryClientTests.cs Outdated
Comment threadsrc/Sentry/ISentryClient.cs Outdated
@bruno-garcia

Copy link
Copy Markdown
Member

🤔 Should I edit the changelog?

Yes please

Comment threadsrc/Sentry/Protocol/EnvelopeItem.cs Outdated
…lock from sentryclient, renamed sentryuserfeedback) and added more unit tests.
@codecov-io

codecov-io commented Oct 28, 2020

Copy link
Copy Markdown

Codecov Report

Merging #559 into main will decrease coverage by 0.21%.
The diff coverage is 73.21%.

Impacted file tree graph

@@ Coverage Diff @@## main #559 +/- ##
==========================================
- Coverage 85.06% 84.85% -0.22% 
==========================================
Files 142 143 +1 Lines 3462 3513 +51 Branches 775 784 +9 ==========================================
+ Hits 2945 2981 +36 - Misses 310 323 +13 - Partials 207 209 +2 
Impacted FilesCoverage Δ
src/Sentry/Extensibility/DisabledHub.cs93.33% <0.00%> (-6.67%)⬇️
src/Sentry/Extensibility/HubAdapter.cs91.30% <0.00%> (-4.16%)⬇️
src/Sentry/Internal/Hub.cs51.35% <0.00%> (-4.54%)⬇️
src/Sentry/SentrySdk.cs93.02% <0.00%> (-4.54%)⬇️
src/Sentry/SentryClient.cs77.77% <68.75%> (-2.69%)⬇️
src/Sentry/Protocol/Envelope.cs97.14% <100.00%> (+0.98%)⬆️
src/Sentry/Protocol/EnvelopeItem.cs63.93% <100.00%> (+3.22%)⬆️
src/Sentry/Protocol/UserFeedback.cs100.00% <100.00%> (ø)
src/Sentry/SentryClientExtensions.cs100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5259ffd...6b628aa. Read the comment docs.

@lucas-zimerman

lucas-zimerman commented Oct 28, 2020

Copy link
Copy Markdown
CollaboratorAuthor

weird, the test worked fine on my computer.
@bruno-garcia I have added the following public callback to SentrySdk because there's a test that enforces me to have all defined SentryClientExtensions also defined into SentrySdk

public static void CaptureUserFeedback(SentryId eventId, string email, string comments, string? name = null)

EDIT:
https://github.com/getsentry/sentry-dotnet/projects/3#card-46143120
and
https://github.com/getsentry/sentry-dotnet/projects/3#card-46044826
could be In progress

@bruno-garciabruno-garcia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Almost there! Left a couple notes.
Thanks!

Comment threadsrc/Sentry/Protocol/UserFeedback.cs Outdated
Comment threadsrc/Sentry/SentryClient.cs Outdated
Comment threadtest/Sentry.Tests/SentryClientTests.cs Outdated
Comment threadtest/Sentry.Tests/SentryClientTests.cs Outdated

@bruno-garciabruno-garcia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Beautiful stuff!! Thanks a lot @lucas-zimerman

Sign up for freeto 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.

User Feedback

3 participants

@lucas-zimerman@bruno-garcia@codecov-io