Account created on 23 August 2009, over 15 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States m.stenta

m.stenta changed the visibility of the branch 3524246-fix-phpstan-errors to hidden.

🇺🇸United States m.stenta

Merged! Thanks everyone for helping with this!

🇺🇸United States m.stenta

We have automated tests now ( 📌 Add basic test coverage Active ), but neither PHP CodeSniffer nor PHPStan are flagging the t() usages.

In order to proceed with this I propose we:

  1. Determine why PHP CodeSniffer / PHPStan are not flagging t().
  2. Tweak testing config so that the usage of t() causes tests to fail.
  3. Use the proposed changes to make the tests pass.
🇺🇸United States m.stenta

We have automated tests now ( 📌 Add basic test coverage Active ) so it would be good to include a test to demonstrate this issue and prevent regressions.

🇺🇸United States m.stenta

We have automated tests now ( 📌 Add basic test coverage Active ) so it would be good to include a test to demonstrate this issue and prevent regressions.

🇺🇸United States m.stenta

We have automated tests now ( 📌 Add basic test coverage Active ) so it would be good to include a test to demonstrate this issue and prevent regressions.

🇺🇸United States m.stenta

We have automated tests now ( 📌 Add basic test coverage Active ) so it would be good to include a test to demonstrate this issue and prevent regressions.

🇺🇸United States m.stenta

We have automated tests now ( 📌 Add basic test coverage Active ) so it would be good to include a test to demonstrate this issue and prevent regressions.

🇺🇸United States m.stenta

We have automated tests now ( 📌 Add basic test coverage Active ) so it would be good to include a test to demonstrate this issue and prevent regressions.

🇺🇸United States m.stenta

We have automated tests now ( 📌 Add basic test coverage Active ) so it would be good to include a test to demonstrate this issue and prevent regressions.

🇺🇸United States m.stenta

Updates:

The entity_reference_validators module put out a new 2.0.0 release that supports Drupal 11! Thanks @pcambra!

@e0ipso made me a maintainer of jsonapi_schema , so we should be able to get a new release of that soon too!

Other than that, everything is ready to go! I opened a draft pull request on GitHub that is ready for review. It includes a TEMP ... commit that removes jsonapi_schema for testing purposes. Once there is a new release of that module, we can update it in this PR and remove that commit.

🇺🇸United States m.stenta

We have automated testing now ( 📌 Add basic test coverage Active ), so this will needs tests too.

🇺🇸United States m.stenta

Talked to @bradjones1 in Slack and we agreed to release 8.x-1.0 after 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review is merged.

These other issues listed in this roadmap can be fixed after that.

🇺🇸United States m.stenta

Specifically, replacing \Drupal:: calls with dependency injection requires changing the constructor arguments of DataDefinitionNormalizer, which would break any downstream classes that extend it. I would like to suggest we do that in 2.x and document it in the release notes as a breaking change. I may add an exception for that particular PHPStan rule so we can make everything else pass in the meantime.

Update: I've added a phpstan-baseline.neon file in 📌 Fix PHPStan errors Active that ignores the \Drupal calls in 8.x-1.x, so all PHPStan tests pass moving forward and we can avoid introducing any new errors. I opened a follow-up issue that removes phpstan-baseline.neon and replaces the \Drupal calls with dependency injection, which can be reviewed and considered for merging into 2.x as a breaking change: 📌 \Drupal calls should be avoided in classes, use dependency injection instead Active .

🇺🇸United States m.stenta

Opened a merge request, and tests all passed.

Marking this as "postponed" on 🌱 [Meta, Plan] Version 2.0 Active .

🇺🇸United States m.stenta

This is ready for merge, but must wait for 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review .

🇺🇸United States m.stenta

Opened a second MR that adds a phpstan-baseline.neon file for ignoring the \Drupal calls. This allows us to acknowledge their existence, and instruct PHPStan to ignore them for the time being, while still flagging any future errors that new changes introduce.

I will open a follow-up issue with a MR for replacing the \Drupal calls with dependency injection, which can be considered for merge into 2.x as a breaking change.

🇺🇸United States m.stenta

Support for float and uri were added in 🐛 Decimal and float fields have a null schema Needs work and 🐛 Error: uri is not a valid type for a JSON document Active .

Updated title and setting to "Needs work" to refactor accordingly.

If we want to continue with this issue we will need to add automated tests for any new additions (now that 📌 Add basic test coverage Active is merged). A merge request would be preferable over patches.

🇺🇸United States m.stenta

Support also needed for "type": "metatag", "type": "uri" and "type": "float"

Support for float and uri were added in 🐛 Decimal and float fields have a null schema Needs work and 🐛 Error: uri is not a valid type for a JSON document Active .

I will follow up in the other issue you opened regarding other types: #3304201: Float, Metatag, URI and layout_section type fields have a empty schema

🇺🇸United States m.stenta

I rebased this branch on top of latest 8.x-1.x (with merged 📌 Add basic test coverage Active ), and added a commit that changes DataDefinitionUndefinedNormalizer::extractPropertyData() so that it returns an array instead of an object. The declared return type is array, so it was wrong for it to return an object, and now that causes a fatal error with Symfony 7+.

Tests are all green (except PHPStan, which will be fixed separately: 📌 Fix PHPStan errors Active ). Setting this back to "Needs Review". Please review this most recent commit and set to RTBC so we can get this merged! :-)

🇺🇸United States m.stenta

I tried replicating this in the new automated tests, but wasn't able to get the code in the MR to trigger, so I think we would need to add more test coverage for this case.

Chatted briefly with @joachim in Slack:

@joachim: Sorry, it’s so long ago I don’t remember
@joachim: My issue came up with a custom data type, not a field type

I'm going to postpone this as "maintainer needs more info" for now. If anyone else runs into this and can provide a failing test to demonstrate it then we can reopen and work on adding support.

🇺🇸United States m.stenta

I think this is ready to merge. All tests are green, and all the context and reasoning is documented thoroughly above and in code comments.

I reached out to @joachim regarding #3332360: Add possible values for data types to see if we can incorporate that too. If I don't hear back I may move forward with this regardless and we can tackle other cases later.

🇺🇸United States m.stenta

I've added tests for this. In doing so, I found that this patch was only working for base fields, not for config fields. It was a simple tweak to support both: instead of checking if the field storage definition is an instanceof ListDataDefinitionInterface, now it checks if the field definition is. Tests are included for both base and config field types now.

I think the easiest solution to this is to simply omit BooleanItem fields from being included in this logic, so that we do not add the oneOf property to them.

I also split the allowed values logic out (and streamlined/documented it) into an allowedValues() method on the DataDefinitionNormalizer class, and then subclassed that for boolean fields to disable allowed values. This removes the need to reference BooleanItem directly in DataDefinitionNormalizer, and it will allow field types that extend boolean to inherit the same default behavior, or override it if they want to.

🇺🇸United States m.stenta

Once we have support for Drupal 11 (see: 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review ), we'll be able to run PHPStan tests via GitLab CI. I've already started working on fixing the existing issues. There aren't many, but some of them should be considered breaking changes. Specifically, replacing \Drupal:: calls with dependency injection requires changing the constructor arguments of DataDefinitionNormalizer, which would break any downstream classes that extend it. I would like to suggest we do that in 2.x and document it in the release notes as a breaking change. I may add an exception for that particular PHPStan rule so we can make everything else pass in the meantime.

🇺🇸United States m.stenta

Rebased this on top of latest 8.x-1.x, which now includes automated tests (via #3257911). Let's see if this breaks anything in the current tests, and then add new tests for this functionality specifically.

🇺🇸United States m.stenta

Confirmed this issue. It happens when there are greater than 2 allowed bundles in an entity reference field.

I created Page, Blog, and Article node types, and added an entity reference field to Articles called test_entity_reference that allows referencing any of them. The expected title on /api/node/article/resource/relationships/test_entity_reference/related/schema should be Article content item, Blog content item, and Page content item, but instead it is Page content item, and Page content item.

I will open a MR with a test for this and @symbioquine's fix.

🇺🇸United States m.stenta

I added general tests for this module in 📌 Add basic test coverage Active , which allowed me to test this. I opened a MR that demonstrates the issue.

In testing this, I found that I had to remove the ResourceType type hint from both line 120 and 159. The automated tests caught both of them.

The GitLab CI test results show the same error:

There was 1 error:
1) Drupal\Tests\jsonapi_schema\Kernel\JsonApiSchemaTest::testJsonApiSchemaRelated
PHPUnit\Framework\Exception: Uncaught PHP Exception TypeError: "Drupal\jsonapi_schema\Controller\JsonApiSchemaController::Drupal\jsonapi_schema\Controller\{closure}(): Argument #1 ($type) must be of type Drupal\jsonapi\ResourceType\ResourceType, string given" at /builds/project/jsonapi_schema/src/Controller/JsonApiSchemaController.php line 159

https://git.drupalcode.org/project/jsonapi_schema/-/jobs/5260475

Now I will push a fix commit, so we can see tests pass.

🇺🇸United States m.stenta

Thanks @stefanlehmann - I'm going to fix this in 🐛 Decimal and float fields have a null schema Needs work , which was originally focused on float field types, but the fix is the same for decimal so I'll do both at once.

🇺🇸United States m.stenta

This also applies to decimal fields. Updating title and description accordingly.

Also, marking this as "Major" because including either of these field types in an entity bundle crashes the schema for that bundle with the following error. This may have changed recently due to upstream changes in Symfony, because I don't think I experienced this originally, but it was also reported in 🐛 Decimal field causes error message on custom entity Active .

The website encountered an unexpected error. Try again later.

TypeError: Symfony\Component\Serializer\Serializer::normalize(): Return value must be of type ArrayObject|array|string|int|float|bool|null, stdClass returned in Symfony\Component\Serializer\Serializer->normalize() (line 161 of /var/lib/tugboat/stm/vendor/symfony/serializer/Serializer.php).

Drupal\jsonapi_schema\Normalizer\ListDataDefinitionNormalizer->normalize(Object, 'schema_json', Array) (Line: 32)
Drupal\jsonapi_schema\Normalizer\FieldDefinitionNormalizer->normalize(Object, 'schema_json', Array) (Line: 161)
Symfony\Component\Serializer\Serializer->normalize(Object, 'schema_json', Array) (Line: 232)
Drupal\jsonapi_schema\Controller\JsonApiSchemaController->Drupal\jsonapi_schema\Controller\{closure}(Array, Object)
array_reduce(Array, Object, Array) (Line: 231)
Drupal\jsonapi_schema\Controller\JsonApiSchemaController->addFieldsSchema(Array, Object) (Line: 210)
Drupal\jsonapi_schema\Controller\JsonApiSchemaController->getResourceObjectSchema(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 638)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 53)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 116)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 90)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 741)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

I've learned a lot about how this module works since I posted the patch in #4. The correct approach is to provide a new normalizer class that declares supported types of decimal and float. I will open a merge request with tests shortly...

🇺🇸United States m.stenta

Thanks @jgaehring! I committed this along with updated tests.

🇺🇸United States m.stenta

Thanks @wimleers! Fixed.

🇺🇸United States m.stenta

I am noticing a similar issue in definitions/relationships/properties/roles/properties/data in the /jsonapi/user/user/resource/schema endpoint (which is not a config entity resource type, but relates to one):

Nevermind, this is supposed to be an array because it's describing a list of all the roles the user has.

🇺🇸United States m.stenta

definitions/attributes/properties/third_party_settings is missing "type": "object"

@symbioquine and @wimleers are correct: type is no longer missing, but it is not correct. It should be object instead of array.

I am noticing a similar issue in definitions/relationships/properties/roles/properties/data in the /jsonapi/user/user/resource/schema endpoint (which is not a config entity resource type, but relates to one):

Notice how it declares "type": "array" instead of object, and includes an items array. Just like in the third_party_settings example.

This does feel related to #3324824: Schema incorrect for config entity "fields" that are Maps and Sequences as @wimleers pointed out ( https://www.drupal.org/project/jsonapi_schema/issues/3324824#comment-148... ). I wonder if we should close this as "outdated" and work on fixing config entity normalization generally over there.

2. Some of the properties like: langcode, theme, region, … are incorrectly under definitions and they should be under definitions/attributes/properties/

This appears to be resolved in 8.x-1.x. Not sure when it was fixed, but they are all under definitions/attributes/properties/ now.

🇺🇸United States m.stenta

I forgot about the "related" schema endpoints (). I pushed some commits that add tests for them, as well as some general test reorganization.

I'm going to go ahead and merge this so that I can start using it to test/fix/merge other bugs/patches in the issue queue.

🇺🇸United States m.stenta

Test required relationships
Test other field types

These are done. Tests are green. I think we can consider this complete! Good enough for a start, anyway. :-)

🇺🇸United States m.stenta

This is where the stdClass object is coming from: https://git.drupalcode.org/project/jsonapi_schema/-/blob/ed083e4274feec8...

It appears that the (object) cast was originally added in this commit (which created the normalizers): 08a4b2f2 (from #3060299: Remove TypeMapper plugins in favor of normalizers ).

... despite the fact that the parent method's docblock declares a return type of array: https://git.drupalcode.org/project/jsonapi_schema/-/blob/08a4b2f2b930a6a...

It appears @ tried to remove this in cd73c2f2, but then reverted that in d30041d3

So we have some things to detangle here... :-/

🇺🇸United States m.stenta

Setting this back to Needs Work because the automated tests I'm working on in 📌 Add basic test coverage Active revealed a bug that will affect Drupal 11 compatibility.

I have rebased on top of the commit in 📌 Add basic test coverage Active that causes the issue to demonstrate the failing test (notably it passes on 8.x-1.x).

There was 1 error:
1) Drupal\Tests\jsonapi_schema\Kernel\JsonApiSchemaTest::testJsonApiSchema
PHPUnit\Framework\Exception: Uncaught PHP Exception TypeError: "Drupal\jsonapi_schema\Normalizer\ComplexDataDefinitionNormalizer::normalize(): Return value must be of type ArrayObject|array|string|int|float|bool|null, stdClass returned" at /builds/project/jsonapi_schema/src/Normalizer/ComplexDataDefinitionNormalizer.php line 51
ERRORS!
Tests: 1, Assertions: 0, Errors: 1.

The declaration of normalize() return types revealed a bug with our current code. When the field module is enabled, there are cases where normalize() attempts to return a stdClass object, which is not allowed. I don't know the exact cause of this yet, but we'll have to figure it out before this can be merged.

This existing issue seems to be closely related: 🐛 Decimal field causes error message on custom entity Active

🇺🇸United States m.stenta

Installing the Field module - I started this but found that simply adding the Field module causes an unusual error (seems related to 🐛 Decimal field causes error message on custom entity Active ). This requires more digging to figure out what's happening.

Ah this turned out to be an issue because I was running tests with the changes necessary for Drupal 11 ( 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review ). When I run against 8.x-1.x this works fine. This is something we will have to fix for D11, though, because that explicitly declares the return types of normalizer methods, and this module returns an invalid type (stdClass) in some cases. That's a separate issue...

🇺🇸United States m.stenta

For what it's worth, Drupal core does not consider constructor changes to be breaking: https://www.drupal.org/about/core/policies/core-change-policies/bc-polic...

However, we may still want to, for the sake of any downstream code that might be extending DataDefinitionNormalizer.

🇺🇸United States m.stenta

Opened a merge request with two commits (but this is built on top of 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review , which in turn is built on [#], so all of those commits are included - they will need to merged before this).

The first commit simply adds drupal/jsonapi_hypermedia as a dev dependency of the module, because PHPStan couldn't find the classes that this module references.

The second commit replaces usage of \Drupal in classes with dependency injection. This change might be considered "breaking" because it changes the constructor arguments for all of the normalizers. Maybe there's another way to do this that is BC. If not, then perhaps we should wait and merge this ahead of a 2.0.0 release and document it as a breaking change (see 🌱 [Meta, Plan] Version 2.0 Active ).

Alternatively, we could remove this line entirely, which would avoid the need for all the changes: https://git.drupalcode.org/project/jsonapi_schema/-/blob/ed083e4274feec8...

🇺🇸United States m.stenta

It seems that phpstan isn't running automatically. I am going to punt that out to a separate issue.

Created a follow-up issue: 📌 Fix PHPStan errors Active

🇺🇸United States m.stenta

I discovered this too while writing automated tests in 📌 Add basic test coverage Active . I had to hard-code those same six entity types as exceptions in the tests to make them pass. If we merge this, we can remove that exception from the tests.

🇺🇸United States m.stenta

Nevermind! 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review will not require a breaking change after all. Disregard my last comment. :-)

🇺🇸United States m.stenta

It's important to understand what this means: if the current output is bad (eg: doesn't meet JSON Schema spec), then these tests are not going to catch those issues. They are primarily going to confirm that the module is working the way it does currently, good or bad.

Adding these tests has already revealed one inconsistency that probably deserves it's own issue. It seems that a lot of config entity types do not declare data.definitions.attributes.type (normally "object") or data.definitions.attributes.description (normally "Entity attributes") in their schema. For example, look at /jsonapi/entity_view_mode/entity_view_mode/resource/schema, and you'll see that data.definitions.attributes only has properties and additionalProperties sub-elements. But if you look at /jsonapi/user_role/user_role/resource/schema it also has type and description.

For now, I've added a condition to the tests to ignore the known problematic entity types, but ideally we can fix that and remove that condition in the future.

🇺🇸United States m.stenta

Tests are passing on both Drupal 10 and 11! 🎉

I think we still need to declare a minimum minor version of Drupal 10 (eg: ^10.1 or ^10.2).

As far as I can tell, the only change that requires this is the one related to https://www.drupal.org/node/3359695 , which was introduced in Drupal 10.1. I pushed an amended commit that updates the version constraint to ^10.1.

This is ready for final review! (But of course must wait for 📌 Add basic test coverage Active to be merged.) If anyone has other concerns/considerations, now is the time to voice them! If I don't hear anything by next week then I will assume the previous RTBC applies, along with the newly restored Drupal 10 support per @dmitry.korhov's feedback.

Thanks all!

🇺🇸United States m.stenta

Rebased on top of the commits from 📌 Add basic test coverage Active so that tests can run. I restored the declaration of Drupal 10 support in jsonapi_schema.info.yml. I also added a commit to ensure tests run on the latest major Drupal version (11). This was disabled in 📌 Add basic test coverage Active because they would fail.

I think we still need to declare a minimum minor version of Drupal 10 (eg: ^10.1 or ^10.2).

🇺🇸United States m.stenta

@dmitry.korhov: I looked closer and I think you're right that it's possible to support both Drupal 10 and 11. I mistakenly assumed that the method signature changes were a breaking change and that it wouldn't be possible to support both.

Relatedly, I've started writing automated tests for this module in a merge request in 📌 Add basic test coverage Active (which leverages 📌 Add .gitlab-ci.yml Active ). These will be very helpful for continued development and bug fixing, and will also allow us to demonstrate that it works in both Drupal 10 and 11.

There is still more work to be done to finish the first pass at automated tests, but I will rebase this issue's MR onto that one so that we can test this in parallel in the meantime.

🇺🇸United States m.stenta

I've started working on adding basic test coverage in MR !15. These first set of tests are passing.

There is more I would like to do, so I'm setting this to "Needs Work", but I want to push what I have now and document some things.

As of right now, I've pushed 5 commits. Here is what each aims to achieve:

  1. Initial JSON:API Schema kernel test. - This just creates an empty kernel test with dependencies, to ensure that simply installing the module works.
  2. Test that /jsonapi/schema is available and has the expected content. - This tests the entrypoint (/jsonapi/schema) schema content. It checks to make sure all expected resource types are listed, and all expected meta data is present.
  3. Test that resource collection schemas are generated. - This focuses on testing the /jsonapi/[entity-type]/[bundle]/collection/schema output. These are relatively simple and standardized across all entity types, as far as I'm aware, so the tests aren't complicated.
  4. Test that resource schemas are generated. - This focused on testing the /jsonapi/[entity-type]/[bundle]/resource/schema output, but only with a focus on the standard/common elements across all resource types.
  5. Add tests for a basic Article node type. - This starts to drill down into more specific details of a single resource type (an article node type without any custom fields). This is where we start to test some of the more specific aspects of attributes and relationships.
  6. Refactor $expected_resources so that it is not order dependent. - I found that the order of resources differed between my local development environment and Drupal.org GitLab, which caused my tests to fail. This commit simply refactors the tests so they aren't dependent on the order of resources.

The next things I want to test are:

  • Installing the Field module - I started this but found that simply adding the Field module causes an unusual error (seems related to 🐛 Decimal field causes error message on custom entity Active ). This requires more digging to figure out what's happening.
  • Test required relationships - Right now there is test to check that "required" attributes are listed (the title attribute of the node is required by default), but there are no tests for required relationships. I will add an entity_reference field to the article node type and make it required to test this.
  • Test other field types - The basic node type allows us to test a few things already (date, string, boolean fields, etc), but in order to test other field types we'll need to create more config fields. So I need to debug the field module installation (above) first.

Important context: Normally test driven development (TDD) would involve writing tests first to lay out the expectations, and then writing the code to meet those expectations. Since all of the current code was written without tests, I am approaching these tests from the other direction. I'm looking at the JSON Schema that the module currently outputs, and writing the expectation tests around that. It's important to understand what this means: if the current output is bad (eg: doesn't meet JSON Schema spec), then these tests are not going to catch those issues. They are primarily going to confirm that the module is working the way it does currently, good or bad. In an ideal world we would have approached this the other way around, but this feels like the best way forward from this point. Adding these tests will allow us to more easily fix other bugs that have been reported, and we can update/improve the tests accordingly as we go. They will also make it easier to confirm that the module works on Drupal 11 (see 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review ).

🇺🇸United States m.stenta

m.stenta made their first commit to this issue’s fork.

🇺🇸United States m.stenta

Pushed a bunch of commits that fix all phpcs warnings and errors.

They are almost entirely comment and whitespace changes.

It's also an opportunity to run phpstan

It seems that phpstan isn't running automatically. I am going to punt that out to a separate issue.

This should be ready to merge.

🇺🇸United States m.stenta

There are two misspelled words flagged by cspell

CSpell: Files checked: 27, Issues found: 5 in 5 files.
The number of distinct unrecognised/misspelled words is 2
------------
millisec
shema
------------

"shema" is fixed by #3324620: Fix plugin ID typo: s/shema/schema/ and "millisec" is fixed by #3246251: Change format "utc-millisec" to "date-time" , so I think we can hold off on fixing those here.

🇺🇸United States m.stenta

Opened an MR that adds a basic .gitlab-ci.yml. It sets OPT_IN_TEST_CURRENT to 0 because we don't support Drupal 11 yet (see 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review ), and sets OPT_IN_TEST_PREVIOUS_MAJOR to 1 to run tests (just linting) against Drupal 10.

composer-lint and eslint tests pass, but cspell and phpcs do not. Setting to Needs Work to work on those.

🇺🇸United States m.stenta

This is a meta issue for migrating the module to do just this, while shedding much of the now-unnecessary schema generation logic.

@bradjones1 Can we rename this issue so that it is more specifically about this refactor, instead of generally about "Version 2.0"?

I think we will need to put out a 2.0.0 release to support Drupal 11 first, before tackling larger refactors.

See: 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review

🇺🇸United States m.stenta

@paul121's PR for farmOS plugin attribute support has been merged into 4.x.

Change record published: https://www.drupal.org/node/3523485

Started a 4.x-attributes branch for this: https://github.com/mstenta/farmOS/tree/4.x-attributes

This includes ConfigEntityType, ContentEntityType, and MigrateSource, which did not have attributes support in Drupal 10.

I've rebased these commits so they are now part of my 4.x-d11 branch, since they are dependent on Drupal 11. So once that's merged, we will be using attributes everywhere instead of annotations.

Changing this back to Needs Work. We can close this once that's merged. Then the only remaining task will be to remove the annotation support in farmOS 5.x, which @paul121 drafted here: https://github.com/paul121/farmOS/commits/4.x-attributes-remove-farm-cor...

🇺🇸United States m.stenta

@dmitry.korhov: I haven't tried it, but it sounds like that is for silencing deprecation notices. In this case it's a fatal error, not a deprecation notice. So I don't think it will help.

I'm inclined to just keep things simple and make a clean break between 1.x and 2.x of this module. Is there any reason not to do that? The upgrade from D10 to D11 will be seamless either way. It just means that sites will need to update their version constraint in composer.json from ^1.0 to ^2.0.

🇺🇸United States m.stenta

Merge request ready for review!

I also noticed that "client_id" was misspelled in config/install/farm_farmlab.settings.ym so I fixed that in a second commit in the same MR.

🇺🇸United States m.stenta

I created a farmOS Integrations module: https://www.drupal.org/project/farm_integration

And opened a merge request that adds FarmLab to the Integration menu (with a dependency on the module).

This is a breaking change (new dependency), so it will require a 2.0.0 release.

🇺🇸United States m.stenta

m.stenta created an issue.

Production build 0.71.5 2024