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

AO3-6232 Include IP, referer, skin in support tickets #4869

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

brianjaustin
Copy link
Member

@brianjaustin brianjaustin commented Jul 1, 2024

Pull Request Checklist

Issue

https://otwarchive.atlassian.net/browse/AO3-6232

Purpose

Adds custom field to Zoho tickets for the submitter's IP address, referer, and the skin they're using at the time of submitting a support ticket

References

The names in this PR will need a final approval from support before merging, to make sure they've created the fields in Zoho as required

Credit

Brian Austin (they/he)

@@ -37,10 +39,16 @@ def load_support_languages
@support_languages = Language.where(support_available: true).default_order
end

def current_skin
Copy link
Contributor

@Bilka2 Bilka2 Jul 1, 2024

Choose a reason for hiding this comment

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

In SkinsHelper#skin_tag, there is also a check for params[:site_skin], e.g. https://test.archiveofourown.org/?site_skin=928.

Because you're essentially replicating the skin setting logic from the #skin_tag method, what do you think about separating that logic out into its own method that can be used both for the support tickets and for #skin_tag?

Copy link
Member

@sarken sarken left a comment

Choose a reason for hiding this comment

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

I am so excited about this -- thank you for working on it! I just have some nitpicks and something we should probably check with Support/PAC about.

app/helpers/skins_helper.rb Show resolved Hide resolved
app/models/feedback.rb Show resolved Hide resolved
app/models/feedback_reporters/support_reporter.rb Outdated Show resolved Hide resolved
spec/controllers/feedbacks_controller_spec.rb Outdated Show resolved Hide resolved
spec/helpers/skins_helper_spec.rb Outdated Show resolved Hide resolved
spec/models/feedback_reporters/support_reporter_spec.rb Outdated Show resolved Hide resolved
Copy link
Member

@sarken sarken left a comment

Choose a reason for hiding this comment

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

Marking changes needed for this comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants