-
Notifications
You must be signed in to change notification settings - Fork 539
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-6751 Fix "Relationships by Character" url for fandoms with periods #5061
base: master
Are you sure you want to change the base?
Conversation
Hi, Emily! Thank you so much for this pull request. It looks pretty good to me, although it would be a good idea to add a Cucumber test so we notice if this breaks again in the future. In the meantime, I've updated the Jira issue status to In Review so no one will mistakenly create a duplicate pull request. If you'd like the ability to comment on, assign, and transition issues in the future, you're welcome to create a Jira account! You can just reply here with the account name and we'll set up the permissions for you. (It makes things a bit easier for us on the organizational side if the Full Name on your Jira account either closely matches the name you'd like us to credit in the release notes or includes it in parentheses, e.g. "Nickname (CREDIT NAME).") Thanks again for contributing! If you have any questions, you can contact us at [email protected]. |
Sure! I'll work on the Cucumber test soon. My Jira is under Emily Wiegand (email: [email protected]). Excited to contribute more! |
It looks like you're not showing up in our User Management section, but no worries -- this happens sometimes! I'll send an invitation to the email you provided, which usually fixes things. |
Feature: Fandom Relationship Tags Link | ||
|
||
Scenario: Click on "Relationships by Character" on a fandom tag containing periods | ||
Given a canonical fandom "Harry Potter - J. K. Rowling" | ||
When I view the tag "Harry Potter - J. K. Rowling" | ||
And I follow "Relationship tags in this fandom" | ||
Then I should not see "The page you were looking for doesn't exist." | ||
And I should see "Relationships by Character" |
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.
I was pretty sure we already had a test for this link and didn't need a new feature file, so I tracked the existing test down: it's in features/other_b/fandoms.feature
(added in #4350). Could you move this test to that file instead? Then I think this will be all done!
Pull Request Checklist
as the first thing in your pull request title (e.g.
AO3-1234 Fix thing
)until they are reviewed and merged before creating new pull requests.
Issue
https://otwarchive.atlassian.net/browse/AO3-6751
Purpose
Ensures period in the fandom tag page for fandoms with periods is properly escaped in the URL. Changed fandom_path(@tag.name) to fandom_path(@tag) in the app/views/tags/shot.html.erb file on line 32
Testing Instructions
Credit
Emily Wiegand (she/her)