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-6667 Languages page performance improvements #4714

Merged
merged 9 commits into from
Feb 18, 2024

Conversation

brianjaustin
Copy link
Member

Pull Request Checklist

Issue

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

Purpose

  • Fetch language counts from Elasticsearch instead of the primary DB
  • Round counts a bit to make temporary discrepancies (in the ones or tens place) less impactful. The counts don't need to be exact anyway
  • Cache counts for 1 day to further reduce load

Copy link
Collaborator

@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.

Just took a quick look and it's possible the failing tests need work_search: true to set up the indexes, like

context "where the user is a maintainer", work_search: true, bookmark_search: true do

Comment on lines 533 to 536
add: Add a language
description: Work Languages
edit: Edit
suggest: Suggest a language
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you keep the original capitalization from the navigation actions? Our navigation should always be title case. (You can, however, drop the "a" if you want.) It also wouldn't hurt to group them under navigation like we do elsewhere to give Translation a bit more context, e.g.

navigation:
approved: Approved
rejected_by_collection: Rejected by Collection
rejected_by_user: Rejected by User
unreviewed_by_collection: Awaiting Collection Approval
unreviewed_by_user: Awaiting User Approval

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good! I think keeping the a sounds better but I can remove it if that's the standard

@sarken sarken merged commit bf72dcf into otwcode:master Feb 18, 2024
26 checks passed
@brianjaustin brianjaustin deleted the AO3-6667 branch April 26, 2024 00:41
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.

2 participants