AO3-7397 AO3-6765 Configurable page for skin previews by nicolacleary · Pull Request #5744 · otwcode/otwarchive

nicolacleary · 2026-04-18T17:03:17Z

Pull Request Checklist

Have you read "How to write the perfect pull request"?
Have you read the contributing guidelines?
Have you added tests for any changed functionality?
Have you added the Jira issue number
as the first thing in your pull request title (e.g. AO3-1234 Fix thing)
Do you have fewer than 5 pull requests already open? If not, please wait
until they are reviewed and merged before creating new pull requests.

Issue

https://otwarchive.atlassian.net/browse/AO3-7397
https://otwarchive.atlassian.net/browse/AO3-6765 (adopted)

Purpose

Redirect to the URL specified in the config with SKIN_PREVIEW_URL when previewing a skin.

Remove the functionality to redirect to the works index of a random tag - this provided no guarantees on the kind of content shown and would sometimes choose tags that no longer existed (resulting in 500 errors).

This PR also adopts changes from #4934 to improve the skin preview message:

Adds the skin ID to the preview tip (rather than making the user copy-paste it themselves)
Adjusts wording in message

Credit

nicolacleary (she/her)
Adopted changes from AO3-6765 #4934 David Bilsky/Ironskink (He/Him)

nicolacleary · 2026-04-18T19:33:29Z

+    flash[:notice] << t(".remove_skin")
+    flash[:notice] << t(".tip")
+    flash[:notice] << ("<a href='#{skin_path(@skin)}' class='action' role='button'>".html_safe + t(".return_to_skin") + "</a>".html_safe)
+    redirect_to "#{ArchiveConfig.SKIN_PREVIEW_URL}?site_skin=#{@skin.id}"


This provides no guarantees that the url is correct (formatting or exists) - e.g. if you configure "/idonotexist" then you would get an error only when you tried to preview a skin.

That's fine, but thanks for noting it!

Bilka2

This is a full review now, but it's just one thing :D

Bilka2 · 2026-05-05T19:13:54Z

+    flash[:notice] << t(".remove_skin")
+    flash[:notice] << t(".tip")
+    flash[:notice] << ("<a href='#{skin_path(@skin)}' class='action' role='button'>".html_safe + t(".return_to_skin") + "</a>".html_safe)
+    redirect_to "#{ArchiveConfig.SKIN_PREVIEW_URL}?site_skin=#{@skin.id}"


That's fine, but thanks for noting it!

Bilka2 · 2026-05-05T19:15:57Z

+    flash[:notice] << t(".skin_title", title: @skin.title)
+    flash[:notice] << t(".remove_skin")
+    flash[:notice] << t(".tip")
+    flash[:notice] << ("<a href='#{skin_path(@skin)}' class='action' role='button'>".html_safe + t(".return_to_skin") + "</a>".html_safe)


Could you use helpers.link_to to create this link HTML so that we can avoid the html_safe call? If you need to remove the role='button' for that, that's fine, it should be removed per AO3-6765 anyway.

Speaking of AO3-6765, the PR for it has been in "Reviewed: Action Needed" without changes for over a year, which makes it adoptable for AD&T volunteers like me. However, if I adopted it now, we'd have loads of merge conflicts between the changes here and the changes for AO3-6765.

Would you be up for adding AO3-6765 into the PR here, since you're already doing the i18n it should just be a matter of changing the text in the locale file and changing the construction of the flash here to not be an array (looks like we usually do multiline flashes with  )? If yes, you have my official AD&T blessing to adopt AO3-6765

I'll look into using the helper as per your first comment.

As for AO3-6765 - I am happy to add that functionality into this PR or a separate one - seems like it's not that complicated in terms of what would change.

I am, however, not quite sure what the procedure is for this. I would rather avoid using the old PR since it has a lot of unrelated linting changes and, given its age, it would take some effort to bring up to date with master (of course it can be done though if necessary) .

Would it be okay to just recreate the code changes in a new commit and update the PR title and description as necessary to add the additional ticket name and credit?

Would it be okay to just recreate the code changes in a new commit and update the PR title and description as necessary to add the additional ticket name and credit?

Yup!

Some extra tests were added to master in AO3-7388 that enforced the old behaviour (redirecting to a random tag for previewing a site skin) Updated so that they check for the value from config.yml '/users/OTW_Translation/works'

in site skin preview. This removes role 'button' but this will be removed as part of AO3-6765 anyways.

Adopt changes from PR otwcode#4934: - Adjust skin preview message text - Include skin id in preview message - Update text on return to skin link (role=button removed in a previous commit)

Use to form the multiline flash message for skin previews, rather than an array which turns it into an unordered list with <ul>

nicolacleary · 2026-05-26T12:16:42Z

The force push was to correct the Jira ticket in some of the commit messages since I got them mixed up. From what I can tell it is not recommended to do this after a review but I decided it would be alright since I was only targeting commits that hadn't been reviewed yet.
Please let me know if I shouldn't do that going forward. Cheers

nicolacleary · 2026-05-26T12:25:38Z

+      t(".skin_title", title: @skin.title),
+      t(".remove_skin"),
+      t(".tip", site_skin_id: @skin.id),
+      helpers.link_to(t(".return_to_skin"), skin_path(@skin), class: "action")
+    ].join('<br />')


Changes in flash message:

Details

Flash before:

Flash using   rather than <ul>:

Flash using   rather than <ul> and wrapping it in :

AO3-6765 says:

The “Return To Skin To Use” link ... should ideally be in a , not a <li>. The role="button" can also be removed, because it is not acting like a button, just a link.

AFAIK I could just do view_context.content_tag(:p, <rest of the line>) and it would work (see screenshots above), but it's a little unclear for me what the desired look of this flash is, given that there are quite a few changes here now. I decided to leave it is as now since it's no longer part of a list.

nicolacleary · 2026-05-26T12:28:56Z

+      t(".remove_skin"),
+      t(".tip", site_skin_id: @skin.id),
+      helpers.link_to(t(".return_to_skin"), skin_path(@skin), class: "action")
+    ].join('<br />')


As far as I can tell, a .html_safe doesn't impact what is rendered in this case, so I omitted it.

github-actions Bot added the Awaiting Review label Apr 18, 2026

nicolacleary added 2 commits April 18, 2026 20:13

AO3-7397 Use t() for skin preview notice text

1bcc21f

AO3-7397 Make skin preview page configurable

c92baf3

nicolacleary force-pushed the AO3-7397_skin_preview_page branch from 779a5ad to c92baf3 Compare April 18, 2026 19:02

nicolacleary commented Apr 18, 2026

View reviewed changes

Comment thread app/controllers/skins_controller.rb Outdated

nicolacleary commented Apr 18, 2026

View reviewed changes

Comment thread config/config.yml Outdated

nicolacleary commented Apr 18, 2026

View reviewed changes

nicolacleary marked this pull request as ready for review April 18, 2026 19:33

AO3-7397 Use prod-ready config value

2278c5b

Bilka2 requested changes May 5, 2026

View reviewed changes

Bilka2 added Reviewed: Action Needed and removed Awaiting Review labels May 5, 2026

Merge branch 'master' into AO3-7397_skin_preview_page

12f800d

github-actions Bot added Coder Has Actioned Review and removed Reviewed: Action Needed labels May 25, 2026

nicolacleary changed the title ~~AO3-7397 Configurable page for skin previews~~ AO3-7397 AO3-6765 Configurable page for skin previews May 26, 2026

nicolacleary added 5 commits May 26, 2026 14:09

AO3-7397 Update site skin tests

f0f19d7

Some extra tests were added to master in AO3-7388 that enforced the old behaviour (redirecting to a random tag for previewing a site skin) Updated so that they check for the value from config.yml '/users/OTW_Translation/works'

AO3-7397 Use link_to instead of html_safe

e58a4e9

in site skin preview. This removes role 'button' but this will be removed as part of AO3-6765 anyways.

AO3-6765 Improve Skin Preview Message

40a8901

Adopt changes from PR otwcode#4934: - Adjust skin preview message text - Include skin id in preview message - Update text on return to skin link (role=button removed in a previous commit)

AO3-6765 Use double quotes in skin preview message

b304d60

AO3-7397 Standardise skin preview flash

1a37970

Use to form the multiline flash message for skin previews, rather than an array which turns it into an unordered list with <ul>

nicolacleary force-pushed the AO3-7397_skin_preview_page branch from ad3ba78 to 1a37970 Compare May 26, 2026 12:14

nicolacleary commented May 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AO3-7397 AO3-6765 Configurable page for skin previews#5744

AO3-7397 AO3-6765 Configurable page for skin previews#5744
nicolacleary wants to merge 9 commits into
otwcode:masterfrom
nicolacleary:AO3-7397_skin_preview_page

nicolacleary commented Apr 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

nicolacleary Apr 18, 2026

Uh oh!

Bilka2 May 5, 2026

Uh oh!

Bilka2 left a comment

Uh oh!

Bilka2 May 5, 2026

Uh oh!

Bilka2 May 5, 2026

Uh oh!

Bilka2 May 5, 2026

Uh oh!

nicolacleary May 10, 2026

Uh oh!

Bilka2 May 20, 2026

Uh oh!

nicolacleary commented May 26, 2026

Uh oh!

nicolacleary May 26, 2026 •

edited

Loading

Uh oh!

nicolacleary May 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

nicolacleary commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Issue

Purpose

Credit

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Bilka2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolacleary commented May 26, 2026

Uh oh!

nicolacleary May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nicolacleary commented Apr 18, 2026 •

edited

Loading

nicolacleary May 26, 2026 •

edited

Loading