Problem/Motivation

CommentItem::mainPropertyName is not overridden, so returns 'value', but 'value' is not a valid property and trying to obtain its value throws 

Uncaught PHP Exception InvalidArgumentException: "Property value is unknown."

Proposed resolution

Override CommentItem::mainPropertyName and return 'cid'.

Remaining tasks

Review.

User interface changes

None.

API changes

None.

Data model changes

None.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because the main property cannot be a non-existent property.
Issue priority Normal because it doesn't affect sites directly, but API users.
Disruption Non disruptive at all.
CommentFileSizeAuthor
#8 2569465-8.patch1.25 KBpenyaskito
#4 2569465-4.patch1.25 KBpenyaskito
#2 2569465-2.patch716 bytespenyaskito

Comments

penyaskito created an issue. See original summary.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Active » Needs review
StatusFileSize
new 2569465-2.patch716 bytes
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work
Issue tags: +Needs tests
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 2569465-4.patch1.25 KB

This may be enough.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Category: Task » Bug report
Issue summary: View changes

Added beta eval. I think this is a bug, not a task.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Issue summary: View changes
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Needs work

The main property should be status, the other properties are essentially computed
Thanks

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new 2569465-8.patch1.25 KB

Oops, thanks. No interdiff, the patch is trivial.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Issue tags: +Quickfix

Tagging as quickfix.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs review » Reviewed & tested by the community

Thanks again

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: 2569465-8.patch, failed testing.

Status: Needs work » Needs review

penyaskito queued 8: 2569465-8.patch for re-testing.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs review » Reviewed & tested by the community

Back to RTBC.

alexpott’s picture

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

Committed 9d11fa4 and pushed to 8.0.x. Thanks!

  • alexpott committed 9d11fa4 on 8.0.x 
    Issue #2569465 by penyaskito: CommentItem must override mainPropertyName...
penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented

Awesome, thanks!

Status: Fixed » Closed (fixed)

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