Problem/Motivation

It's become clear over the past 24 hours that FieldPropExpression and friends (everything in src/PropExpressions) is hard to grok for people.

Eventually we'll need docs, initially unit tests can serve as docs.

In this first pass, there is one key goal: unit tests to go from PHP object representation to string representation and vice versa should pass — because once that's working, contributors won't lose time anymore due to bugs in that low-level foundation. (Which currently still is 99% identical to the PoC from ~1.5 month ago.)

Out of scope: making all code in src/PropExpressions beautiful/elegant/DRY/… — the purpose is only the bold bit above, because that A) unblocks other issues/MRs, B) keeps scope tight.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

  • API addition: single StructuredDataPropExpression::from(…) method to avoid if/else logic to invoke the appropriate class-specific method.

Data model changes

  • 🥳 No more mysterious invisible whitespace in the string representation. Result: less frustration, and the sort order in test expectations now actually makes sense.
  • The string representation of Field(Type)ObjectPropsExpression no longer lists the mapped object props separated by , (comma + space), but only by , (comma).
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Wim Leers created an issue. See original summary.

wim leers’s picture

Assigned: wim leers » Unassigned
Status: Active » Needs work

To be continued on Monday.

Unless somebody wants to continue where I ended 🤓

wim leers’s picture

Assigned: Unassigned » wim leers
wim leers’s picture

Title: Unit tests for PropExpressions » Unit tests for PropExpressions + single <code
Priority: Normal » Major
Issue summary: View changes
wim leers’s picture

Title: Unit tests for PropExpressions + single <code » Unit tests for PropExpressions + single StructuredDataPropExpression::from(…) method
wim leers’s picture

Title: Unit tests for PropExpressions + single StructuredDataPropExpression::from(…) method » Unit tests for PropExpressions to go to/from string representation + single StructuredDataPropExpression::from(…) method
Priority: Major » Critical
Issue summary: View changes
wim leers’s picture

Assigned: wim leers » Unassigned
Status: Needs work » Needs review
wim leers’s picture

Issue summary: View changes

wim leers’s picture

Massive review from @larowlan on the MR — all addressed!

wim leers’s picture

Status: Needs review » Reviewed & tested by the community

@f.mazeikis@gmail.com approved this after I addressed @larowlan's review 👍

  • Wim Leers committed 0ba2ceeb on 0.x
    Issue #3451626 by Wim Leers, larowlan, f.mazeikis: Unit tests for...
wim leers’s picture

Status: Reviewed & tested by the community » Fixed

  • Wim Leers committed 0ba2ceeb on sdc_prop_choices
    Issue #3451626 by Wim Leers, larowlan, f.mazeikis: Unit tests for...

Status: Fixed » Closed (fixed)

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