Problem/Motivation

The "Display all values in the same row" setting for the "Multiple field settings" field plugin settings doesn't work properly for views REST displays. All fields are printed for each row instead of one per row when the option is unchecked.

Reproduction

  1. Start from the Standard installation profile.
  2. Set the cardinality field of the image field on article to unlimited
  3. Create some article nodes with multiple images
  4. Enable the "RESTful Web Services" module
  5. Create a new view with a "REST export" display
  6. Set row display to fields
  7. Add the image field and uncheck the "Display all values in the same row" checkbox

Actual Result

[
  {
    "title": "pretium a ipsum et; sodales",
    "images": "  <img src=\"https://dfrkb.ply.st/sites/default/files/field/image/600x300.png\" width=\"600\" height=\"300\" alt=\"Nullam scelerisque enim nec arcu dapibus mollis. Vestibulum soda.\" typeof=\"foaf:Image\" />\n\n,   <img src=\"https://dfrkb.ply.st/sites/default/files/field/image/600x300_0.png\" width=\"600\" height=\"300\" alt=\"arcu dapibus mollis. Vestibulum soda.\" typeof=\"foaf:Image\" />\n\n"
  },
  {
    "title": "pretium a ipsum et; sodales",
    "images": "  <img src=\"https://dfrkb.ply.st/sites/default/files/field/image/600x300.png\" width=\"600\" height=\"300\" alt=\"Nullam scelerisque enim nec arcu dapibus mollis. Vestibulum soda.\" typeof=\"foaf:Image\" />\n\n,   <img src=\"https://dfrkb.ply.st/sites/default/files/field/image/600x300_0.png\" width=\"600\" height=\"300\" alt=\"arcu dapibus mollis. Vestibulum soda.\" typeof=\"foaf:Image\" />\n\n"
  }
]

Expected Result

[
  {
    "title": "pretium a ipsum et; sodales",
    "images": "  <img src=\"https://dfrkb.ply.st/sites/default/files/field/image/600x300.png\" width=\"600\" height=\"300\" alt=\"Nullam scelerisque enim nec arcu dapibus mollis. Vestibulum soda.\" typeof=\"foaf:Image\" />\n\n"
  },
  {
    "title": "pretium a ipsum et; sodales",
    "images": "  <img src=\"https://dfrkb.ply.st/sites/default/files/field/image/600x300_0.png\" width=\"600\" height=\"300\" alt=\"arcu dapibus mollis. Vestibulum soda.\" typeof=\"foaf:Image\" />\n\n"
  }
]

Proposed resolution

Add a test to check if the setting is being honored. Debug and fix the output.

Remaining tasks

Debug and fix.

User interface changes

none

API changes

none

Data model changes

none

CommentFileSizeAuthor
#14 2556029-14-reroll-only.patch4.32 KBJaesin
#7 2556029-7-complete.patch4.34 KBgeertvd
#7 2556029-7-test.patch3.52 KBgeertvd
#5 2556029-5.patch843 bytesgeertvd
config-Site_UUID_8fffa5f2-60d7-4db7-a83c-60bfb87986c0.tar_.gz27.63 KBJaesin

Comments

Jaesin created an issue. See original summary.

geertvd’s picture

geertvd commented
Title: REST views "Display all values in the same row" doesn't limit the value to one per row. » REST views uncheking "Display all values in the same row" doesn't limit the value to one per row.
Issue summary: View changes

I can reproduce this issue, updated the IS with steps to reproduce that describe the problem better.

geertvd’s picture

geertvd commented
Title: REST views uncheking "Display all values in the same row" doesn't limit the value to one per row. » REST views unchecking "Display all values in the same row" doesn't limit the value to one per row.
geertvd’s picture

geertvd commented
Assigned: Unassigned » geertvd
geertvd’s picture

geertvd commented
Issue tags: +Needs tests
StatusFileSize
new 2556029-5.patch843 bytes

This fixes that, still needs tests though, will work on that now.

geertvd’s picture

geertvd commented
Status: Active » Needs review
geertvd’s picture

geertvd commented
StatusFileSize
new 2556029-7-test.patch3.52 KB
new 2556029-7-complete.patch4.34 KB

Added tests

geertvd’s picture

geertvd commented
Assigned: geertvd » Unassigned
geertvd’s picture

geertvd commented
Issue tags: -Needs tests
geertvd’s picture

geertvd commented
Issue tags: +VDC

The last submitted patch, 7: 2556029-7-test.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented

Nice work! The fix looks perfect for me, just curious whether this fixes the actual problem of @Jaesin

Jaesin’s picture

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

Nice work indeed.

  1. I started a new test site with patch from #7.
  2. I increased the carnality of the image field to unlimited.
  3. I added a couple of nodes populating the images field with 2 images each.
  4. I created a new view and a REST display using fields and added the image field.
  5. I unchecked the "Display all values in the same row" option, saved and tested.
    The results were as expected: 
    [
  {
    "title": "2 lkajsd  lksjad lkjasd lksja d",
    "field_image": "  <img src=\"https://dfr7v.ply.st/sites/default/files/field/image/900x450_0.png\" width=\"900\" height=\"450\" alt=\"j ljqwlkje\" typeof=\"foaf:Image\" />\n\n"
  },
  {
    "title": "2 lkajsd  lksjad lkjasd lksja d",
    "field_image": "  <img src=\"https://dfr7v.ply.st/sites/default/files/field/image/600x300_0.png\" width=\"600\" height=\"300\" alt=\"kaljsd lasjkd\" typeof=\"foaf:Image\" />\n\n"
  },
  {
    "title": "lajsd ljaslkdj ",
    "field_image": "  <img src=\"https://dfr7v.ply.st/sites/default/files/field/image/600x300.png\" width=\"600\" height=\"300\" alt=\",masnd\" typeof=\"foaf:Image\" />\n\n"
  },
  {
    "title": "lajsd ljaslkdj ",
    "field_image": "  <img src=\"https://dfr7v.ply.st/sites/default/files/field/image/900x450.png\" width=\"900\" height=\"450\" alt=\"lasd;kas\" typeof=\"foaf:Image\" />\n\n"
  }
]
  6. I checked the "Display all values in the same row" option, saved and tested.
    The results were as expected: 
    [
  {
    "title": "2 lkajsd  lksjad lkjasd lksja d",
    "field_image": "  <img src=\"https://dfr7v.ply.st/sites/default/files/field/image/900x450_0.png\" width=\"900\" height=\"450\" alt=\"j ljqwlkje\" typeof=\"foaf:Image\" />\n\n,   <img src=\"https://dfr7v.ply.st/sites/default/files/field/image/600x300_0.png\" width=\"600\" height=\"300\" alt=\"kaljsd lasjkd\" typeof=\"foaf:Image\" />\n\n"
  },
  {
    "title": "lajsd ljaslkdj ",
    "field_image": "  <img src=\"https://dfr7v.ply.st/sites/default/files/field/image/600x300.png\" width=\"600\" height=\"300\" alt=\",masnd\" typeof=\"foaf:Image\" />\n\n,   <img src=\"https://dfr7v.ply.st/sites/default/files/field/image/900x450.png\" width=\"900\" height=\"450\" alt=\"lasd;kas\" typeof=\"foaf:Image\" />\n\n"
  }
]
Jaesin’s picture

Jaesin commented
StatusFileSize
new 2556029-14-reroll-only.patch4.32 KB

This is just a re-roll because of #2557519: Remove many usages SafeMarkup::checkPlain() and replace with Html::escape(). No changes at all.

  • alexpott committed f6163c3 on 8.0.x 
    Issue #2556029 by geertvd, Jaesin, dawehner: REST views unchecking "...
geertvd’s picture

geertvd commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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