Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#29131 closed Cleanup/optimization (fixed)

Make the error message for ArrayField more user and translation-friendly

Reported by: Vlada Macek Owned by:
Component: contrib.postgres Version: 2.0
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: yes

Description

There's an 'item_invalid' error message
"Item %(nth)s in the array did not validate: "

that has problems:

  • While this message is shown to end user, it contains a 0-based index, which can be a challenge to explain, even in english...
  • I believe "list" would be a better term instead of "array" for general public. The same goes for my language.
  • Additional message is concatenated right after this, so the trailing space is needed in translations too. But on Transifex and many other i18n tools the space is not apparent.

What about something like this:

"Item %(nth)s (count from the left) in the list is not valid:"

Change History (7)

comment:1 Changed 6 years ago by Tim Graham

Summary: Error message of ArrayField is unfriendly and has excessive trailing spaceMake the error message for ArrayField more user and translation-friendly
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

I think indexing from 1 instead of 0 is an improvement.

Changing array to list might be okay, but what about all the messages in the ArrayField model field that use "array".

About the addition of "(count from the left)"... I think that would be better added in translational only for languages (if any?) where counting in lists happens from the right?

I see your point about the trailing space -- the translated messages I checked don't have the trailing space. I guess the idea would be to modify contrib.postgres.utils.prefix_validation_error() to add the space instead.

comment:2 Changed 6 years ago by Vlada Macek

Changing array to list might be okay, but what about all the messages in the ArrayField model field that use "array".

Quickly peeking... I'd say this could be the only message observable by the end user.

About the addition of "(count from the left)"... I think that would be better added in translational only for languages (if any?) where counting in lists happens from the right?

I proposed such suboptimal wording rather to point out the non-technical people might not be even prepared to count items from either side.

The resulting concatenated message for ArrayField(base_field=models.EmailField) looks like this:

<field name>: Item %(nth)s in the array did not validate: Enter valid e-mail address

Personally I'd like the value itself to be mentioned:

<field name>: One of the values in the list (%(value)s) did not validate: Enter valid e-mail address

I guess the idea would be to modify contrib.postgres.utils.prefix_validation_error() to add the space instead.

Exactly what I found out.

Thank you.

Last edited 6 years ago by Vlada Macek (previous) (diff)

comment:3 Changed 5 years ago by Tim Graham <timograham@…>

In b33f10d:

Refs #29131 -- Made ArrayField error messages index from 1 instead of 0.

comment:4 Changed 5 years ago by Tim Graham <timograham@…>

In 31ce1f74:

Refs #29131 -- Fixed space handling in ArrayField's item_invalid message.

comment:5 Changed 5 years ago by Tim Graham

Resolution: fixed
Status: newclosed

comment:6 Changed 4 years ago by Pavel Savchenko

This feature came as a surprise to us when we upgraded from 2.0 to 2.1, as we were relying on 0-index (i.e we were adding +1 manually to ).

I guess it's too late for release notes (though we could mention it in 3.1?), though we could also mention that it's 1-indexed in the related documentation here:

https://docs.djangoproject.com/en/3.0/ref/contrib/postgres/fields/#django.contrib.postgres.fields.ArrayField.base_field

What do you think, shall we create a separate ticket for documenting this?

comment:7 Changed 4 years ago by Mariusz Felisiak

I'm not sure what kind of change or documentation improvement we could do. These patches changed only error messages. Index and slice transforms still use 0-based indexing.

Note: See TracTickets for help on using tickets.
Back to Top