Improve processors to support an optional key argument

Created on 26 April 2024, 8 months ago
Updated 23 May 2024, 7 months ago

Problem/Motivation

Auto plugin should do regular processing, but needs a configurable attribute/slot name. For that we need to improve our processor pipeline to add support for this optional name. The name should be optional, to keep BC for 2.x processors.

📌 Task
Status

Fixed

Version

3.0

Component

Code

Created by

🇦🇹Austria fago Vienna

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @fago
  • 🇦🇹Austria fago Vienna
  • Assigned to roderik
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    OK. I'm fairly sure what should roughly be done here, but the code is too abstract for me to see if it's small or large.

    I'll just start by hacking the optional name argument into CustomElementGenerator::process() + the addToElement() implementations, starting with DefaultContentEntityProcessor + DefaultFieldItemListProcessor.

    And then see if I have questions / whether I already see what the actual practical behavior change is and how AutoCeFieldFormatter::build() should be simplified (??) when it works...

  • Pipeline finished with Success
    8 months ago
    Total: 171s
    #165829
  • Pipeline finished with Success
    8 months ago
    Total: 176s
    #165831
  • Issue was unassigned.
  • Status changed to Needs review 8 months ago
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    ( OK that wasn't actually very much / didn't cause any breakage. I just needed to change/test the code, in order to see things clearly. )

    Created draft change record. Created MR with question about whether we should add an extra annotation.

  • 🇦🇹Austria fago Vienna

    hmm, this is not so "optional" as I had thought of it.

    Modifying all Processor classes can be done before updating the custom_elements module to version 3.x, without impact. Immediately after updating the module, all previously unmodified Processor classes will break.

    That's not good, since it could result into severe upgrade headache. e.g. at lupus_decoupled we want to support custom_elements 2 and 3 in parallel until 3 is stable. Also that means custom code must be changed when updating, what makes it complicated. I think we should check and find a way to avoid this.

    >The name should be optional, to keep BC for 2.x processors.
    We really need this BC I think.

  • 🇦🇹Austria fago Vienna

    Took a stab on how it could look like in a fully BC way. What do you think about that?

    https://git.drupalcode.org/project/custom_elements/-/merge_requests/49/d...

  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    That proposal looks good.

    Because of the other potentially-breaking changes we are introducing, I was getting lost a little on whether we want to prevent breakage for sites on the 3.x upgrade. But personally I always tend to favor 'not breaking things' so I agree. (And yes, this change would break things in a 'wider' way than the others.)

    I amended the Change Record. I also added a note that things still break on upgrade for Processor classes which extend custom_elements processor classes -- but that's not an official contract.

    I might want to sneak a sentence into CustomElementProcessorWithKeyInterface comments, referring to the reason for needing to implement it (see Change Record). But I don't think I have push rights to your MR in the main repo, so I'll do that in another MR.

    So:
    RTBC for code
    Needs Review for change notice.

  • Pipeline finished with Success
    8 months ago
    Total: 172s
    #166226
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    Oh -- I guess you wanted me to add your code to MR 48 because that's the one referenced here.

    So I did, and I did sneak in that extra sentence. (I believe that there's plenty of room for code readers to get confused between Processors and Formatters and CE displays etc - so a minimal reference to "why?" is IMHO warranted.)

    https://git.drupalcode.org/project/custom_elements/-/merge_requests/48/d...

  • Status changed to Needs work 8 months ago
  • 🇦🇹Austria fago Vienna
  • Pipeline finished with Success
    8 months ago
    Total: 239s
    #166464
  • Status changed to Needs review 8 months ago
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    Merged 3.x (includes 📌 Separate configuration of CE field formatters and display components Fixed ), implemented feedback, build is green again.

  • Pipeline finished with Success
    8 months ago
    Total: 205s
    #166491
  • Pipeline finished with Canceled
    8 months ago
    Total: 138s
    #166611
  • Pipeline finished with Running
    8 months ago
    #166615
  • Pipeline finished with Success
    8 months ago
    Total: 205s
    #168661
    • fago committed c5cf33b1 on 3.x authored by roderik
      Issue #3443794 by roderik: Improve processors to support an optional key...
  • Status changed to Fixed 8 months ago
  • 🇦🇹Austria fago Vienna

    I resolved conflicts with gitlab and merged results! Thanks!

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

Production build 0.71.5 2024