I am facing the same issue wherein the `Body` is missing and hence it is not possible to Try Out POST requests from Swagger.
hardikpandya → created an issue.
This could go as a separate issue but since D10 compatibility is talked here, I would like to mention that I am having a hard time to make this module work with Drupal 10. I have tried this on a fresh Drupal 10 installation but the issue persists.
Post adding the keys, secret and channel, I have created a template(attached is the configuration of it). For ease, I have added just the title field to the template.
When I create a new article and have Publish to Apple News checked, the node gets saved but get an error Apple News: INVALID_DOCUMENT. In the database log, I see this message:
User notice: Array ( [code] => 400 [message] => HTTP/2 400 [response] => stdClass Object ( [errors] => Array ( [0] => stdClass Object ( [code] => INVALID_DOCUMENT [keyPath] => Array ( ) [message] => Exception while processing the article with Message Expected to find an object with property ['metadata'] in path $ but found 'com.fasterxml.jackson.databind.node.NullNode'. This is not a json object according to the JsonProvider: 'com.jayway.jsonpath.spi.json.JacksonJsonNodeJsonProvider'. ) ) ) ) in C:\laragon\www\drupal10\vendor\chapter-three\apple-news-api\src\PublisherAPI\Base.php on line 197 in ChapterThree\AppleNewsAPI\PublisherAPI\Base->triggerError() (line 288 of C:\laragon\www\drupal10\vendor\chapter-three\apple-news-api\src\PublisherAPI\Base.php)
Is there anything that I have missed?
This is covered as part of work in https://www.drupal.org/project/applenews/issues/3388755 🐛 D10 compatibility follow-up Needs review
I have tried this patch on one of my projects and it works. I am leaving this for others review and to be marked as RTBC.
hardikpandya → created an issue.
Kindly ignoe the above patch, attaching here the updated patch.
Here is the patch to fix this.
hardikpandya → created an issue.
Here is the patch for this.
hardikpandya → created an issue.
hardikpandya → created an issue.
hardikpandya → made their first commit to this issue’s fork.
hardikpandya → made their first commit to this issue’s fork.
An additional Recommended Modules item was in the Table of Contents. I have updated that and added hyperlinks to the table of contents items.
hardikpandya → made their first commit to this issue’s fork.
hardikpandya → made their first commit to this issue’s fork.
Tested by applying the patch and can confirm there are no more phpcs issues reported. RTBC +1
hardikpandya → made their first commit to this issue’s fork.
I have added the table of contents which was missing in the README file.
hardikpandya → made their first commit to this issue’s fork.
hardikpandya → made their first commit to this issue’s fork.
Added core_version_requirement in the info file. The README file normally can have more than 80 characters and that warning could be ignored.
hardikpandya → made their first commit to this issue’s fork.
I was not able to apply the patch in #2. Got this error:
error: patch failed: log_stdout.info.yml:2
error: log_stdout.info.yml: patch does not apply
This would be covered as part of phpcs fixes in https://www.drupal.org/project/log_stdout/issues/3349806 📌 Fix the issues reported by phpcs Needs review
This would be part of phpcs fixes and a single ticket should be created to fix all phpcs issues rather than breaking it up. Updating the details and tags for this issue.
Closing as this is duplicate of https://www.drupal.org/project/log_stdout/issues/3339853 📌 Drupal 10 compatibility Needs review
RTBC for the patch provided by @samit.310@gmail.com. The MR removes the comment entirely which is inappropriate and hence marking this as Needs Work.
Rebased and added hyperlinks to the table of content items.
The MR is currently not mergeable and would need a rebase.
Rebased and fixed all phpcs issues. Please consider MR-4 for review.
This would be covered as part of https://www.drupal.org/project/menu_item_role_access/issues/3293109 🐛 Fix the issues reported by phpcs Needs review where more work for fixing all phpcs issues is being done. Closing this!
Referring to the MR, there are still issues pending.
FILE: tests/src/Functional/PermissionsTest.php
-------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------
68 | ERROR | The array declaration extends to column 96 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------------------------------------------------------------
FILE: menu_item_role_access.module
--------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------------
24 | WARNING | Unused variable $config.
--------------------------------------------------------------------------------------
FILE: src/MenuItemRoleAccessLinkTreeManipulator.php
-----------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
-----------------------------------------------------------------------------------------------------------
79 | WARNING | MenuLinkContent::load calls should be avoided in classes, use dependency injection instead
91 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-----------------------------------------------------------------------------------------------------------
Time: 406ms; Memory: 10MB
Created MR that fixes all phpcs issues reported.
With Drupal 10 here and reduced support for Drupal 8 going forward, I feel the info file should reflect the same. I have updated the core_version_requirement accordingly. Attached is the interdiff.
I have fixed a few more phpcs issues. Attached is the interdiff.
I still see the below issue reported which I am not sure of how to fix.
FILE: tests/src/Kernel/TomeGeneratorEntityEmbedTest.php
-------------------------------------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------------------------------------------------------------------------
30 | ERROR | Do not disable strict config schema checking in tests. Instead ensure your module properly declares its schema for configurations.
-------------------------------------------------------------------------------------------------------------------------------------------------The last applied patch is not applying.
error: patch failed: src/Plugin/WebformHandler/SharpSpringCrmLeadHandler.php:105
error: src/Plugin/WebformHandler/SharpSpringCrmLeadHandler.php: patch does not applyI will work on fixing this.
The patch fixes reported phpcs issues. RTBC +1
The patch fixes reported phpcs issues. RTBC +1
The constants should be preferably declared in an Interface compared to a Class.
In that case, this can be marked RTBC.
The README file is missing Table of Contents and Maintainers section.
I agree on the points for README and mixed typehint. I did found a couple of other phpcs issues pending related to DI.
FILE: src/Discovery/HookEventsDiscovery.php
---------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
---------------------------------------------------------------------------------------------
64 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
66 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
---------------------------------------------------------------------------------------------I feel if this is happening, the same should be done for the README file too.
PHPCS issues are fixed. RTBC +1
hardikpandya → made their first commit to this issue’s fork.
This fixes all phpcs issues reported. Marking RTBC.
hardikpandya → created an issue.
hardikpandya → made their first commit to this issue’s fork.
I am referring to Related Modules details added in hook_help.
The section Related Modules is normally meant for the module page on drupal.org and not hook_help.
hardikpandya → made their first commit to this issue’s fork.
The patch fixes all phpcs issues. Marking RTBC!
The patch fixes all phpcs issues. Marking RTBC!
Looks good. Marking RTBC!
Looks good. Marking RTBC!
The patch applies clean and fixes all phpcs issues. Marking RTBC!
The patch fixes all phpcs issues reported in the issue. Marking RTBC!
The configure in the info file seems to be incorrect, it is just pointing to the entity list view. Ideally, it should point to any configuration/admin settings form the module implements.
I do not see any such configuration form here, hence, the need for this issue.
Keeping this as Active for the module maintainer to check and take action on this.
There were a few phpcs issues left unaddressed. I have fixed them, provided the interdiff.
hardikpandya → made their first commit to this issue’s fork.
There are no more phpcs issues reported. Marking RTBC and created MR.
hardikpandya → made their first commit to this issue’s fork.
Looks Good. Marking as RTBC!
hardikpandya → created an issue.
There are no more t() call issues reported by phpcs. Marking as RTBC!
hardikpandya → made their first commit to this issue’s fork.
There are still procedural t() calls in src/Plugin/Field/FieldWidget/XnumberWidget.php and src/Tests/XnumberFieldTest.php. Please fix them!
The patch isn't getting applied. Please work on making it compatible.
Also, @sahil.goyal it is a good practice to provide an interdiff from the previous patch submitted by a contributor to easily identify the changes done by you. See: https://www.drupal.org/node/1488712 →
MR 5 looks good. Marking as RTBC.
All phpcs issues are fixed. Marking as RTBC.
Also, created an MR for easy merging.
hardikpandya → made their first commit to this issue’s fork.