Problem/Motivation

TextLongItem and StringLongItem both inherit the 'max_length' settings from their base field. But its not used anywhere but it is configurable in the user interface.

Proposed resolution

Remove the setting form TextLongItem and StringLongItem.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#11 drupal-stringitembase-2380391-11.patch4.76 KBmrharolda
#10 drupal-stringitembase-2380391-10.patch4.78 KBmrharolda
#1 stringitem-max-length-2380391-1.patch2.39 KBwebflo

Comments

webflo’s picture

webflo commented
Status: Active » Needs review
StatusFileSize
new stringitem-max-length-2380391-1.patch2.39 KB
webflo’s picture

webflo commented

This blocks #2068655: Entity fields do not support case sensitive queries

Status: Needs review » Needs work

The last submitted patch, 1: stringitem-max-length-2380391-1.patch, failed testing.

Status: Needs work » Needs review

webflo queued 1: stringitem-max-length-2380391-1.patch for re-testing.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Priority: Normal » Critical

So it is a critical then cuz it blocks a critical

chx’s picture

chx commented
Status: Needs review » Reviewed & tested by the community

Thanks, this is a very simple and nice fix.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue addresses a critical bug and is allowed per https://www.drupal.org/core/beta-changes. Committed d8776af and pushed to 8.0.x. Thanks!

  • alexpott committed d8776af on 8.0.x 
    Issue #2380391 by webflo: Fix storage settings for TextLongItem and...
yched’s picture

yched commented
Title: Fix storage settings for TextLongItem and StringLongItem » Fix storage settings StringLongItem

Nice - however, contrary to what the issue title says, this was only about StringLongItem ?
TextLongItem doesn't have the issue (doesn't inherit from TextItem), and the patch touched nothing there.

Also, the config schema for storage settings needs to be updated accordingly - opened #2381079: Adjust storage_settings schema for string_long field type

mrharolda’s picture

mrharolda commented
StatusFileSize
new drupal-stringitembase-2380391-10.patch4.78 KB

I might be completely wrong here, but StringLongItem extends StringItem, but removes the max_length restrictions? Shouldn't that be the other way araound: the least restrictive as base?

... or an abstract StringItemBase as base, and extend that into StringItem and StringLongItem?

I've attatched a patch of how I think it should be, but again: I'm a nOOP, pun intended ;)

mrharolda’s picture

mrharolda commented
StatusFileSize
new drupal-stringitembase-2380391-11.patch4.76 KB

Oops ... missed a spot. The comment was still referring to numeric fields.

chx’s picture

chx commented

I'd recommend opening a new issue cos the patch on a fixed issue will get overlooked.

mrharolda’s picture

mrharolda commented

Done: #2383277: StringLongItem should not extend StringItem

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.