8.x-1.0 has been released! 🎉
https://www.drupal.org/project/jsonapi_schema/releases/8.x-1.0 →
m.stenta → changed the visibility of the branch 8.x-1.x to hidden.
m.stenta → changed the visibility of the branch 3524246-fix-phpstan-errors to hidden.
Merged! Thanks everyone for helping with this!
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:
- Determine why PHP CodeSniffer / PHPStan are not flagging
t()
. - Tweak testing config so that the usage of
t()
causes tests to fail. - Use the proposed changes to make the tests pass.
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.
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.
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.
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.
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.
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.
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.
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.
We have automated testing now ( 📌 Add basic test coverage Active ), so this will needs tests too.
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.
Specifically, replacing
\Drupal::
calls with dependency injection requires changing the constructor arguments ofDataDefinitionNormalizer
, 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
.
Opened a merge request, and tests all passed.
Marking this as "postponed" on 🌱 [Meta, Plan] Version 2.0 Active .
This is ready for merge, but must wait for 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review .
m.stenta → created an issue.
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.
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.
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 →
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! :-)
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.
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.
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 theoneOf
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.
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.
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.
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.
Tests are green!
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.
Added a test for this.
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.
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...
m.stenta → created an issue.
Thanks @jgaehring! I committed this along with updated tests.
Thanks @wimleers! Fixed.
m.stenta → created an issue.
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.
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 underdefinitions/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.
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.
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. :-)
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... :-/
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
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...
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
.
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...
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
m.stenta → created an issue.
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.
Adding these tests has already revealed one inconsistency that probably deserves it's own issue.
Oh I wonder if this is related: #3324769: Schema incorrect for config entity types with multi-part IDs (field_storage_config, field_config, entity_view_mode, entity_form_mode, entity_view_display and entity_form_display) →
Nevermind! 📌 Automated Drupal 11 compatibility fixes for jsonapi_schema Needs review will not require a breaking change after all. Disregard my last comment. :-)
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.
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!
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
).
@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.
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:
- Initial JSON:API Schema kernel test. - This just creates an empty kernel test with dependencies, to ensure that simply installing the module works.
- 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. - 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. - 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. - 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.
- 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 (thetitle
attribute of the node is required by default), but there are no tests for requiredrelationships
. I will add anentity_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 ).
m.stenta → made their first commit to this issue’s fork.
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.
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.
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.
m.stenta → created an issue.
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
@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-attributesThis includes
ConfigEntityType
,ContentEntityType
, andMigrateSource
, 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...
@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.
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.
m.stenta → created an issue.
Rebased onto new 2.x branch.
m.stenta → created an issue.
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.
m.stenta → created an issue.
Thanks @pcambra!!
PR is ready for review: https://github.com/farmOS/farmOS/pull/960
Merged into 2.x.