Opened 4 years ago

Last modified 11 months 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 2 years ago.
149.diff (4.6 KB) - added by markoheijnen 2 years ago.
149.2.diff (1.0 KB) - added by yoavf 11 months ago.

Download all attachments as: .zip

Change History (18)

comment:1 @nacin3 years ago

  • Keywords has-patch commit added

comment:2 @nacin3 years ago

  • Keywords has-patch commit removed

Patch was added to the wrong ticket.

comment:3 @nacin3 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.

comment:4 @Japh2 years ago

  • Cc japh@… added

comment:5 @Japh2 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.

@Japh2 years ago

comment:6 @Japh2 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.

comment:7 @nbachiyski2 years ago

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

comment:8 @markoheijnen2 years ago

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

comment:9 @dllh2 years ago

  • Cc daryl@… added

@markoheijnen2 years ago

comment:10 @markoheijnen2 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.

comment:11 @markoheijnen2 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.

@yoavf11 months ago

comment:12 @yoavf11 months ago

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

comment:13 @yoavf11 months ago

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

Last edited 11 months ago by yoavf (previous) (diff)

comment:14 @markoheijnen11 months 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() ?

comment:15 @yoavf11 months ago

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

Note: See TracTickets for help on using tickets.