Opened 7 years ago

Last modified 2 years ago

#149 new defect

Do not allow empty translations

Reported by: zodiac1978 Owned by: somebody
Milestone: 1.0 Priority: blocker
Version: Component: General
Keywords: has-patch 2nd-opinion Cc: japh@…, daryl@…, yoav@…


If you have a string which has Singular and Plural and you have just translated the Singular or the plural of the string, the whole string is marked as translated (green).

It should remain untranslated until the second part is translated too.

(Seen on translate.wordpress.com)

Attachments (3)

149.1.diff (3.0 KB) - added by Japh 5 years ago.
149.diff (4.6 KB) - added by markoheijnen 5 years ago.
149.2.diff (1.0 KB) - added by yoavf 4 years ago.

Download all attachments as: .zip

Change History (19)

#1 @nacin
6 years ago

  • Keywords has-patch commit added

#2 @nacin
6 years ago

  • Keywords has-patch commit removed

Patch was added to the wrong ticket.

#3 @nacin
6 years ago

  • Milestone set to 1.0
  • Priority changed from major to blocker
  • Summary changed from If just Singular is translated, but not plural the string is marked as translated to Do not allow empty translations

The real fix for this ticket is #112. I am going to repurpose this one.

#4 @Japh
5 years ago

  • Cc japh@… added

#5 @Japh
5 years ago

Patch 149.1.diff is an attempt to start getting validation on. It looks like the validation methods are already there, at least partly, and now they're being used.

Better error messages are needed (#226), and a very rudimentary first go at that is also in this patch.

5 years ago

#6 @Japh
5 years ago

Note: I left the word "invalid" in the error messages because changes to the error messages break the assertInvalidRedirect test, which should probably not be so fragile / specific.

#7 @nbachiyski
5 years ago

I agree the test could be better. Do you have a better idea how to tell if we got this specific error?

#8 @markoheijnen
5 years ago

Hey Japh, are you fine that this patch getting committed or is something else needed?

#9 @dllh
5 years ago

  • Cc daryl@… added

5 years ago

#10 @markoheijnen
5 years ago

Changed some of the logic to have a better readable string. Still not really happy with it but #226 can be used for that.
Also did some small cleanup around the code.

#11 @markoheijnen
5 years ago

Patch 149.diff has been commited [749]. Only thing that needs to happen here is that errors also need to show up when there are plurals for a string.

4 years ago

#12 @yoavf
4 years ago

  • Cc yoav@… added
  • Keywords has-patch 2nd-opinion added

#13 @yoavf
4 years ago

149.2.diff should not allow submission of strings with missing plural translations.

Last edited 4 years ago by yoavf (previous) (diff)

#14 @markoheijnen
4 years ago

I'm still a bit mixed about the patch. Unsure if the code is in the right place. Couldn't it be in GP::$translation_warnings->check() ?

#15 @yoavf
4 years ago

Happy to consider a better location, but warnings are just warnings and can easily be discarded by validators.

This ticket was mentioned in Slack in #glotpress by ocean90. View the logs.

2 years ago

Note: See TracTickets for help on using tickets.