- Issue created by @grasmash
- Merge request !818Fix Dynamic List component layout settings persistence β (Open) created by grasmash
- π¬π§United Kingdom catch
This is the second issue I've seen in the past month in the XB queue that appears to violate the issue etiquette guidelines on AI generated content https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... β
I don't think it is respectful of reviewer time to post unreviewed code like this, especially without a clear disclaimer.
- π¬π§United Kingdom catch
This sounds like it's re-implementing a sub-set of views functionality from whole cloth, whereas as described, I would expect a feature like this to create a view under the hood. That way all the existing views plugins (mini pager, infinite scroll, full pager), responsive layouts etc. are available. The UI for creating the listing could be similar, just a case of what gets created and saved. This was the concept behind https://www.drupal.org/project/simpleviews β back in the day.
- π¬π§United Kingdom longwave UK
As you can hopefully see from the 121 comments I added to the MR, there are some severe problems with this. A summary:
- The overall approach is not really explained anywhere - firstly why is this a separate component source plugin and not just a single implementation of e.g. a block?
- There are trivial XSS attacks and escaping problems because HTML is built directly by concatenating strings instead of using render arrays
- New concepts such as "field binding expressions" have been invented with no explanation
- Entire files have been conjured and added to the repo with seemingly no reason
- Existing tests and the module install function have been modified with no explanation
Was this MR even self-reviewed? At least some of these fundamental problems should have been spotted by the author, surely?
- πΊπΈUnited States effulgentsia
Re #6, the Overview tab of the MR says:
π€ Generated with Claude Code
But yeah, that's easy to miss if you just click the MR link from the issue and then immediately click the Changes tab (which is what I do >90% of the time) without reading the Overview tab.
Therefore, I'm adding the disclaimer to the issue title.
I wonder if we should add more specifics to https://www.drupal.org/docs/develop/issues/issue-procedures-and-etiquett... β to standardize on how to disclose this info. Should it be in the issue title? The issue summary? A label on the MR? I think a label on the MR would be great if d.o. added MR labels to the MR link that's at the bottom of the issue summary.
- π¬π§United Kingdom catch
@effulgentsia I was referring to https://www.drupal.org/project/experience_builder/issues/3515646#comment... β¨ Add automated image optimization to image component Active
- πΊπΈUnited States effulgentsia
@catch: sorry, i can see how my comment of "I'm not aware of any other AI-generated MR in the XB issue queue at the moment" could be read as questioning your "This is the second issue" comment. That's not how I intended it. I'm aware of β¨ Add automated image optimization to image component Active , but the MR currently on there is no longer the initial AI-generated one, so my comment just meant that there's no others in the queue right now for me to re-title, as far as I know.
As to your other question, I think the issue summary was written by @grasmash (or him in collaboration with other people), but from a Product Management rather than Engineering perspective, and from a "this is the desired feature-set / capabilities" perspective rather than a "here's what Drupal building blocks already exist, here's the path to get from them to the desired UX" perspective. I agree with the "Needs issue summary update" tag to get closer to the latter, but I find the currently written Product-oriented write-up a necessary and helpful starting point from which to get to the Engineering-oriented one.