Portland, OR
Account created on 23 November 2007, over 16 years ago
  • Senior Software Architect at Phase2Β  …
#

Merge Requests

Recent comments

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

#2 also resolves this with Gin and Layout Builder. I'm bumping to Major is it makes LB un-usable. A test may be tricky since this seems to only be surfaced by Contrib modules...

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Moving to NR and adding a patch for use with composer.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Thanks all! I'll cut a new release shortly.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Since this is critical, do we want to commit the current fix (all tests are passing) and deal with the fallout/ideal fix in a follow-up?

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Patch for composer. There's a branch in gitlab, but I'm not seeing the button to open a MR.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Thanks all!

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Thanks!

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Thanks all!

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Thanks all!

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ“Œ | Message | Gitlab CI
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Thanks!

πŸ“Œ | Message | Gitlab CI
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Done!

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Thanks all!

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Thanks!

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

It wasn't, but in order to commit, the failing unit test needs to be updated to pass with this change.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

There should be a test to demonstrate this issue/fix.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

These have been taken care of in other issues I think.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Thanks all!

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

This actually isn't reproducable on a vanilla D10.1 install.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Sounds good. Here's a patch and a PR :)

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

I think this makes sense to commit now if it fixes the issue, since this is critical. It is a bit disconcerting though that neither the issue here, or the change over in πŸ› Invalid context for call to FieldDefinitionInterface->isDisplayConfigurable() Fixed were caught by tests. I'd recommend we do a follow-up to add test coverage for this so we don't regress it again in the future.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

#6 appears to have been addressed already in the module. I've merged the other branch above into the issue branch, and am attaching a patch for use with composer.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

That code is from a very early port of the module to 8.x (commit 68bbb58).

There's one more use where the module hard codes display to that method call in field_permissions_jsonapi_entity_field_filter_access that should be updated to view I guess.

(cross post!)

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Wow, that's bizarre. I wonder how this was ever working since BaseFieldDefinition is just checking the definition array (which is presumably also just using view and edit):

  public function isDisplayConfigurable($display_context) {
    return $this->definition['display'][$display_context]['configurable'] ?? FALSE;
  }
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Hmm, these aren't failing locally for me.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

As noted above, the failing test still needs to be addressed in 8.2.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

This PR (and attached patch for use with composer) adds this option to the field formatter. The UI might be confusing, so we could perhaps use the form states api to hide or show the options as needed...

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Adding a patch for use with composer.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

I've made an attempt to implement the direction in #2. Attaching a patch for use with composer.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Adding a patch file for use with composer.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR
πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

I've fixed the error in #11 (it was due to final classes not being mock-able in unit tests, so the fix was to mock the interface instead.)

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

The fatal error noted in #8 has been resolved in the Webform module since 2021 (#3213630), so perhaps tests are being run with a very old version of that module somehow?

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

I've tested #3 and created an MR with the same code. In manual testing it seems to work with D10.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

jhedstrom β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States jhedstrom Portland, OR

Thanks all!

Production build https://api.contrib.social 0.61.6-2-g546bc20