Repository navigation
AO3-4122 Prevent duplicate translation languages for admin posts #6003
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -102,6 +102,7 @@ en: | |
| invalid_permissions: Comment permissions are invalid. | ||
| translated_post_id: | ||
| must_be_posted_first: cannot be posted before the original post | ||
| translation_already_exists: already has a translation in this language | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm making sure this message is clear with Translation and will let you know what they say! |
||
| block: | ||
| attributes: | ||
| blocked: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,6 +78,61 @@ | |
| end | ||
| end | ||
|
|
||
| describe "#translated_post_id" do | ||
| let(:admin_post) { create(:admin_post) } | ||
| let(:language) { create(:language) } | ||
| let(:translation) do | ||
| build(:admin_post, translated_post: admin_post, language: language) | ||
| end | ||
| let(:message) { "already has a translation in this language" } | ||
|
|
||
| context "when the original post has a posted translation in the language" do | ||
| before do | ||
| create(:admin_post, translated_post: admin_post, language: language) | ||
| end | ||
|
|
||
| it "adds an error to the translated post" do | ||
| expect(translation).to be_invalid | ||
| expect(translation.errors[:translated_post_id]).to include(message) | ||
| end | ||
| end | ||
|
|
||
| context "when the original post has a draft translation in the language" do | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue only mentions published translations, but this implementation also blocks the case where the existing translation is still a draft. Drafts are shared between all admins, otherwise a second translation in the same language could be posted while the draft is pending.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, the issue predates the existence of drafts, but this is the behavior I'd expect now that we have them. Thanks for including drafts in test coverage! |
||
| before do | ||
| create(:admin_post, :draft, | ||
| translated_post: admin_post, language: language) | ||
| end | ||
|
|
||
| it "adds an error to the translated post" do | ||
| expect(translation).to be_invalid | ||
| expect(translation.errors[:translated_post_id]).to include(message) | ||
| end | ||
| end | ||
|
|
||
| context "when another post has a translation in that language" do | ||
| before do | ||
| other_post = create(:admin_post) | ||
| create(:admin_post, translated_post: other_post, language: language) | ||
| end | ||
|
|
||
| it "does not add an error" do | ||
| expect(translation).to be_valid | ||
| end | ||
| end | ||
|
|
||
| context "when updating the existing translation" do | ||
| let!(:existing) do | ||
| create(:admin_post, translated_post: admin_post, language: language) | ||
| end | ||
|
|
||
| it "does not add an error" do | ||
| existing.title = "Updated translation title" | ||
|
|
||
| expect(existing).to be_valid | ||
| end | ||
| end | ||
| end | ||
|
|
||
| describe "#set_published_at" do | ||
| before { freeze_time } | ||
|
|
||
|
|
||
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.
Disabled this cop instead of adding a unique index on (translated_post_id, language_id). Production likely has duplicates already (that's what this issue is about), so the index would need a data cleanup first, and that felt out of scope here. The validation itself runs the same query either way and uses the existing index on translated_post_id.