AO3-6726 Separate languages table into work languages and locale languages by pmonfort · Pull Request #5923 · otwcode/otwarchive

pmonfort · 2026-07-08T02:40:02Z

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-6726

Purpose

Separates the languages table, currently used for both works and admin/i18n features, into two independent tables:

languages — used exclusively for works
locale_languages — used for admin/i18n: locales, admin posts, admin post tags, abuse reports, and support/feedback forms

What this PR does:

Creates the locale_languages table by cloning the existing languages tablATE TABLE ... LIKE+INSERT INTO ... SELECT), preserving IDs so existing language_id` foreign keys work without data migration.
Updates associations on Locale, AdminPost, and AdminPostTag to belongsign_key: :language_id, reusing the existing FK column to avoid data migrations on those tables (per redsummernight's suggestion on related PR. Also removes an orphaned belongs_to :language from ArchiveFaq (no FK existed in the table).
Updates views for abuse reports, support/feedback forms, admin posts, and locanguageinstead ofLanguage. The abuse report and feedback controllers now query LocaleLanguage` for available support languages (the models store language as a str
Adds admin management for locale languages (index, new, create, edit, update)ndit policy (role-based permitted attributes), views, routes, and i18n keys. Adds a "Languages" dropdown to the admin nav with links to "Work Languages" and "Locale Lan
Removes "Support available" and "Abuse support available" checkboxes from thee those fields now belong to locale languages. The columns remain in the languages table but are no longer exposed through the UI or policy.
Restricts work language creation/editing to support and superadmin roles available to translation). The work languages index remains public (no authorization on index).
Adds a fallback rake task (rake After:populate_locale_languages_table) to rfrom languages if needed after deploy. The migration already copies the data; this task is only needed if the migration data needs to be re-synced.

`LanguagePolicy` changes:

create? and update? restricted to support and superadmin (previously also and policy_and_abuse)
Permitted attributes limited to name, short, sortable_name (no more support/

`LocaleLanguagePolicy` roles:

index?: translation, support, superadmin, policy_and_abuse
create?: translation, superadmin
update?: translation, superadmin, support, policy_and_abuse (with role-ses: support can only edit support_available, policy_and_abuse can only edit abuse_support_available)

Testing Instructions

Testing instructions are in the Jira issue.

References

Original draft PR: AO3-6726 separate languages used for works from other uses of languages #5004 (Cesium-Ice), used as reference for controller spec struc

Credit

Cesium-Ice
Pablo Monfort (he/him)

sarken

Mostly nitpicks! I realize some of them come from copying the language code, and I don't expect you to go back and change anything in the existing language code (this pull request is big enough!), but it doesn't hurt to make a few tweaks on the locale language side of things.

sarken · 2026-07-10T02:50:20Z

+  LANGUAGE_EDIT_ACCESS = %w[superadmin support].freeze
+  LANGUAGE_CREATE_ACCESS = %w[superadmin support].freeze


I think we can simplify this policy since we no longer need to split access for create and edit, much less by field. (If we ever need to split it again, we can just dig this old code out of the file's history.)

sarken · 2026-07-10T02:53:55Z

+  LANGUAGE_EDIT_ACCESS = %w[superadmin translation support policy_and_abuse].freeze
+  LANGUAGE_CREATE_ACCESS = %w[superadmin translation].freeze


Our naming conventions in policies are all over the place, but I think as long as we're here, switching to EDIT_ROLES and CREATE_ROLES would be shorter and consistent with the names in some of our other policies.

sarken · 2026-07-10T03:05:51Z

@@ -0,0 +1,3 @@
+<h2 class="heading"><%= t(".heading") %></h2>


For both this file and /app/views/locale_languages/new.html.erb, could you add the standard HTML comments to this file and change the key to page_heading to better match the majority of our pages, please? Just like:

 <h2 class="heading"><%= t(".page_heading") %></h2>     <%= render "form" %>

sarken · 2026-07-10T03:07:30Z

+<!--/descriptions-->
+
+<!--subnav-->
+<p class="navigation actions" role="navigation">


Can you remove the ARIA role from this?

sarken · 2026-07-10T05:59:06Z

+    authorize LocaleLanguage
+    @locale_language = LocaleLanguage.new(permitted_attributes(LocaleLanguage))
+    if @locale_language.save
+      flash[:notice] = t("locale_languages.successfully_added")


I think we usually prefer lazy lookup for the flash messages in our controllers unless we're reusing the same message in multiple actions. With that change, the key for the success messages can probably just be .success.

sarken · 2026-07-29T05:30:58Z

+    (Admin::VALID_ROLES - %w[superadmin translation support policy_and_abuse]).each do |role|
+      context "when logged in as an admin with #{role} role" do
+        let(:admin) { create(:admin, roles: [role]) }
+
+        it "redirects with error" do
+          fake_login_admin(admin)
+          get :index
+
+          it_redirects_to_with_error(root_url, "Sorry, only an authorized admin can access the page you were trying to reach.")
+        end
+      end
+    end
+
+    %w[translation superadmin support policy_and_abuse].each do |role|
+      context "when logged in as an admin with #{role} role" do
+        let(:admin) { create(:admin, roles: [role]) }
+
+        it "renders the index template" do
+          fake_login_admin(admin)
+          get :index
+          expect(response).to render_template("index")
+        end
+      end
+    end


Similar to the above, the "an action only authorized admins can access" shared example might be helpful here.

Done, except for update. The support and policy_and_abuse roles raise UnpermittedParameters there, so the shared example's success check doesn't work. Left that one as is.

sarken · 2026-07-29T05:33:55Z

+      end
+    end
+
+    context "when the locale language does not exist" do


It might be good to note here and in similar places and the admin is authorized, since I assume the behavior is different for non-admins and unauthorized admins.

sarken · 2026-08-31T06:39:21Z

    $stdout.flush
  end

+  # Fallback for the migration that copies the languages table.


I think this would be better in language_tasks.rake. We usually use after_tasks.rake for things that need to be run after deploy and never again, but if I understand correctly, this seems more like an optional thing to use in the future. (tbh, I don't think we'll need it, since the goal of this separation is to allow the tables to diverge over time. But it's probably not a bad idea to have it just in case something unexpected happens.)

We could also use some tests for this and some output that tells us if a locale language couldn't be updated or created.

Moved it to language_tasks.rake and made it a proper re-sync. It now updates existing rows too, reports created/updated/failed counts, and prints each row that couldn't be saved. Added tests 👍

sarken · 2026-08-31T06:58:27Z

-  german = Language.find_or_create_by(short: "DE", name: "Deutsch", sortable_name: "Deutsch", support_available: true, abuse_support_available: true)
-  Locale.create(iso: "de", name: "Deutsch", language: german)
+  Language.find_or_create_by(short: "DE", name: "Deutsch", sortable_name: "Deutsch")
+  LocaleLanguage.default


This is super nitpicky, but a blank line between 59 and 60 might help with readability, since we're switching from Language to LocaleLanguage. (Same with lines 73 and 74 for the Persian step.)

sarken · 2026-08-31T07:02:11Z

+      locale_language = LocaleLanguage.find_or_create_by(short: "en", name: "English")
+      Locale.find_by(iso: "en") || Locale.create(iso: "en", name: "English (US)", language_id: locale_language.id, main: 1)
+    else
+      Locale.find_by(iso: "en") || Locale.create(iso: "en", name: "English (US)", language_id: language.id, main: 1)


Just wondering why the switch from set_base_locale here?

Two reasons. set_base_locale's first line is LocaleLanguage.find_by(short: ArchiveConfig.DEFAULT_LANGUAGE_SHORT), but this rescue branch is the "ArchiveConfig didn't work" path, so calling it here would just fail again. It also runs during rake db:migrate itself, so on a database that hasn't run this PR's CreateLocaleLanguages migration yet , locale_languages doesn't exist and set_base_locale's LocaleLanguage lookup would crash the migration run. The table_exists? fork keeps the old language_id behavior for that window and switches to the locale language afterwards.

pmonfort · 2026-09-03T05:03:45Z

Thanks for your feedback! I updated the code according to the comments, let me know if it looks good to you.

github-actions Bot added Has Migrations Contains migrations and therefore needs special attention when deploying Awaiting Review labels Jul 8, 2026

pmonfort force-pushed the AO3-6726 branch 3 times, most recently from b692618 to 2bb1649 Compare July 8, 2026 03:23

marcus8448 mentioned this pull request Jul 28, 2026

AO3-7542 Add permalink and blog_lang to Akismet comment checks #5944

Merged

5 tasks

pmonfort added 5 commits August 10, 2026 20:24

AO3-6726 Create LocaleLanguage model, migration, and rake task

17ca910

AO3-6726 Move locale associations from Language to LocaleLanguage

bd3842c

AO3-6726 Add LocaleLanguages controller, policy, and routes

defd37f

AO3-6726 Add LocaleLanguages views and i18n keys

0b4640e

AO3-6726 Update specs and cucumber features

adc8283

pmonfort force-pushed the AO3-6726 branch from 2bb1649 to adc8283 Compare August 10, 2026 23:28

AO3-6726 Add language alias to AdminPost for Akismet compatibility

9123805

pmonfort force-pushed the AO3-6726 branch from 56914d2 to 9123805 Compare August 11, 2026 03:18

AO3-6726 merge master and fix conflicts

8510202

sarken requested changes Aug 31, 2026

View reviewed changes

sarken added Reviewed: Action Needed and removed Awaiting Review labels Aug 31, 2026

AO3-6726 Address pull request comments

ab1d078

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

AO3-6726 Fix flaky rake task spec

021d44e

pmonfort requested a review from sarken September 4, 2026 02:53

		LANGUAGE_EDIT_ACCESS = %w[superadmin support].freeze
		LANGUAGE_CREATE_ACCESS = %w[superadmin support].freeze

		LANGUAGE_EDIT_ACCESS = %w[superadmin translation support policy_and_abuse].freeze
		LANGUAGE_CREATE_ACCESS = %w[superadmin translation].freeze

		@@ -0,0 +1,3 @@
		<h2 class="heading"><%= t(".heading") %></h2>

Uh oh!

Conversation

pmonfort commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Checklist

Issue

Purpose

What this PR does:

LanguagePolicy changes:

LocaleLanguagePolicy roles:

Testing Instructions

References

Credit

Uh oh!

sarken 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pmonfort commented Sep 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pmonfort commented Jul 8, 2026 •

edited

Loading

`LanguagePolicy` changes:

`LocaleLanguagePolicy` roles: