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.

Sign up for GitHub

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

Jump to bottom

New Feedback Form #424

Merged
merged 27 commits into from
Feb 24, 2022
Merged

New Feedback Form #424

merged 27 commits into from
Feb 24, 2022

Conversation

tomasstrba
Copy link
Contributor

@tomasstrba tomasstrba commented Feb 9, 2022 •
edited
Loading

Task/Issue URL: https://app.asana.com/0/72649045549333/1201099482686712/f
Tech Design URL: -
CC:

Description:
New improved way how we collect feedback reports and breakage reports

Steps to test this PR:
Test website breakage report

  1. Compile and run with Release configuration
  2. Visit this url
  3. Open feedback form (Help->Send Feedback or right menu -> Send Feedback)
  4. Select "Website is broken"
  5. Pick some issue
  6. Submit the form
  7. Visit Kibana and make sure there is a new item you submitted

Test app breakage report

  1. Open feedback form (Help->Send Feedback or right menu -> Send Feedback)
  2. Select "DuckDuckGo App is broken"
  3. Type something into the text box
  4. Submit
  5. Visit macOS Feedback and make sure there is a new task with category "Something is broken"

Test feature request report
Same as previous but select "Feature request" and make sure Feedback Type in Asana is "Feature Request"

Test feature request report
Same as previous but select "General feedback" and make sure Feedback Type in Asana is "Other"

Testing checklist:

  • Test with Release configuration
Internal references:

Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM

@samsymons samsymons self-assigned this Feb 9, 2022
samsymons
samsymons reviewed Feb 9, 2022
View reviewed changes
Copy link
Collaborator

@samsymons samsymons left a comment

Choose a reason for hiding this comment

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

Just a couple questions after skimming the code, haven't ran this locally yet.

DuckDuckGo/Feedback and Breakage/Model/WebsiteBreakage.swift Outdated Show resolved Hide resolved
DuckDuckGo/Feedback and Breakage/View/FeedbackViewController.swift Outdated Show resolved Hide resolved
DuckDuckGo/Statistics/PixelEvent.swift Show resolved Hide resolved
@tomasstrba
Copy link
Contributor Author

tomasstrba commented Feb 11, 2022 •
edited
Loading

@samsymons, thanks for you review! I am assigning this to me, since there are few changes coming out of copy and product review. I will let you know when this is ready for a final review

@tomasstrba tomasstrba assigned tomasstrba and unassigned samsymons Feb 11, 2022
@tomasstrba tomasstrba assigned samsymons and unassigned tomasstrba Feb 17, 2022
@tomasstrba
Copy link
Contributor Author

@samsymons, this is ready for a final review. :)

@samsymons
Copy link
Collaborator

@tomasstrba Will begin my review on this tomorrow, thanks for the patience!

samsymons
samsymons reviewed Feb 23, 2022
View reviewed changes
DuckDuckGo/Feedback and Breakage/Model/FeedbackSender.swift
Comment on lines +48 to +50
case .bug: return "1199184518165816"
case .featureRequest: return "1199184518165815"
case .other: return "1200574389728916"
Copy link
Collaborator

@samsymons samsymons Feb 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Part of me wishes that the Asana ID was handled on the backend side so that we can change it without breaking old releases. Part of that is because I don't know what happens if someone outside the company sees this API call and starts spamming it with random Asana project IDs. But, I think this is fine 🙂

Copy link
Contributor Author

@tomasstrba tomasstrba Feb 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I was thinking about this and we will probably take care of old releases on backend side.

Agree the Asana ID doesn't look good but it basically doesn't matter. We could use names instead but if someone wants to spam random values, they will.

samsymons
samsymons approved these changes Feb 23, 2022
View reviewed changes
Copy link
Collaborator

@samsymons samsymons left a comment

Choose a reason for hiding this comment

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

LGTM! Hard to find any problems with the code at all, this is all really nicely structured and super easy to read. Ship it!

@samsymons samsymons assigned tomasstrba and unassigned samsymons Feb 23, 2022
@tomasstrba tomasstrba merged commit f2ce3e9 into develop Feb 24, 2022
@tomasstrba tomasstrba deleted the tom/new-feedback branch February 24, 2022 22:22
@samsymons
Copy link
Collaborator

🎉

samsymons added a commit that referenced this pull request Feb 25, 2022
@samsymons
Merge branch 'develop' into sam/logins-authentication
1db7eb0 
# By Sam Macbeth (2) and others
# Via GitHub
* develop:
  Top autofill (#432)
  Option to add new notes or edit existing is disabled (#446)
  Use our own autoconsent fork (#444)
  New Feedback Form (#424)
  Update privacy dashboard (#440)
  Fix crash when background tabs trigger cookie popup (#439)
  Update clickToLoadConfig.json (#435)
  rename weakAssign to assign(to:onWeaklyHeld:) (#442)
  Improve Safari favorite importing (#436)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
samsymons added a commit that referenced this pull request Mar 2, 2022
@samsymons
Merge branch 'develop' into sam/autofill-cards-and-identities
22762cb 
# By Tomas Strba (6) and others
# Via GitHub (1) and Tomas Strba (1)
* develop:
  Logins+ authentication (#438)
  Update privacy dashboard to main (#451)
  Version 0.19.1
  Remove mouse event listeners on browser close (#448)
  Find in Page Hotfix + refactoring of forced unwrapping (#450)
  Memory leak fixed (#449)
  Version 0.19.0
  Top autofill (#432)
  Option to add new notes or edit existing is disabled (#446)
  Use our own autoconsent fork (#444)
  New Feedback Form (#424)
  Update privacy dashboard (#440)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	DuckDuckGo/SecureVault/View/PasswordManager.storyboard
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samsymons samsymons samsymons approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tomasstrba @samsymons