AO3-6978 Create and fill new table for import urls#5659
AO3-6978 Create and fill new table for import urls#5659luis-pabon-tf wants to merge 21 commits intootwcode:masterfrom
Conversation
Used co-authors work as reference when making this migration Co-Authored-By: Éric B. <896729+ceithir@users.noreply.github.com> Co-Authored-By: Melissa Jiménez <96195033+gmjimenezc@users.noreply.github.com>
…mine Co-Authored-By: Melissa Jiménez <96195033+gmjimenezc@users.noreply.github.com>
github-actions
bot
added
Has Migrations
brianjaustin
left a comment
brianjaustin
left a comment
There was a problem hiding this comment.
Thanks! One thing for the test. Also, could you add a test for the rake task?
| t.string :minimal, null: false | ||
| t.string :minimal_no_protocol_no_www, null: false | ||
| t.string :no_www, null: false | ||
| t.string :with_www, null: false | ||
| t.string :with_http, null: false | ||
| t.string :with_https, null: false | ||
| t.string :encoded, null: false | ||
| t.string :decoded, null: false |
There was a problem hiding this comment.
We may want indexes on some of these later, but since indexes aren't totally free, I'm fine not doing that for now to see how MariaDB optimises as-is (still better than a query starting with a wildcard 😅 )
There was a problem hiding this comment.
Yeah, I was thinking that it would be better to add indexes once the new tables are usable to see which columns would actually benefit.
spec/models/imported_url_spec.rb
Outdated
| expect(url.minimal).to eq(formatter.minimal) | ||
| expect(url.minimal_no_protocol_no_www).to eq(formatter.minimal_no_protocol_no_www) | ||
| expect(url.no_www).to eq(formatter.no_www) | ||
| expect(url.with_www).to eq(formatter.with_www) | ||
| expect(url.with_http).to eq(formatter.with_http) | ||
| expect(url.with_https).to eq(formatter.with_https) | ||
| expect(url.encoded).to eq(formatter.encoded) | ||
| expect(url.decoded).to eq(formatter.decoded) |
There was a problem hiding this comment.
To make it easier to see what each of these represents, could you make these be raw strings (so for example expect(url.minimal).to eq("http://www.trickster.org/llwyden/misc/cracked.html") instead of the first line)?
github-actions
bot
added
Coder Has Actioned Review
and removed
Reviewed: Action Needed
labels
2 participants
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-6978
Purpose
Step 1 of moving imported URLs to a new table to store all searchable variations.
Testing Instructions
Besides the steps on the Jira, you can check that the task works by checking
ImportedUrl.laston the Rails console.References
Are there other relevant issues/pull requests/mailing list discussions? If not, you can remove this section.
AO3-6979 follows up on this one
AO3-6591 Is the parent both tickets belong to
Credit
What name and pronouns should we use to credit you in the Archive of Our Own's Release Notes?
Luis Pabon (they/he)