🇮🇳India @diwakar07

Reviewed MR !449.
The changes look good. It successfully adds content type column in the Recent Content table.

@anjaliprasannan Let's try to hide/remove the label of the "Content type" field in the view, as it seems to be breaking the design of the table.
Moving to NW for now.

Reviewed MR !442, @anjaliprasannan added few comments for you.
The changes look good for the Person Profile content type (/people) page.
Attached is the SS for reference.

I think we need to implement the same for all the content types having a listing page.
Please update the MR accordingly.
Moving for NW.

diwakar07 → made their first commit to this issue’s fork.

Hi,
Created an MR that adds the Analytics and Accessibility Tools recipe as suggested recipe in the composer file of drupal_cms_seo_basic and drupal_cms_seo_tools.

Please review.

diwakar07 → made their first commit to this issue’s fork.

To improve the current implementation of Shortcuts, a good starting point could be to retain the essential options, such as 'Extend your site' and 'Setup a redirect,' while removing other shortcuts related to recipes which it is already available under the 'Create' menu to eliminate redundancy.

The tests appear to be passing now.

Please review.

+1
This looks good to me as well.

The default event is being rendered properly.
The file structure is now more logical and well-organized.
Attaching SS for reference.

Moving to RTBC!

Hi,
Created an MR to add Simple XML Sitemap defaults for Event content type.
It successfully adds 'Events' to be included in /admin/config/search/simplesitemap/entities.

Please review.

Working on it.

Not an issue! @phenaproxima
I’d be happy to help review the MR if that works for you.

+1
I was able to reproduce the bug reported.
After the implementation of the content moderation state, it seems more appropriate to grant the 'Content Editor' role permissions to edit, view, and update unpublished content and transition it to another state.

Working on it.

Hi,
Created an MR to update the site email to the email provided by the user during site setup.

Please review.

Working on it.

Thanks @atul_ghate,
The MR !26 looks good to me.
It successfully resolves the issue. I am able to see the field label when setting display to "Inline".

Moving to RTBC!

Hi,
Reviewed the sticky actions implementation and it looks great and consistent for entity add/edit forms as well as config forms, it adds the action buttons in the sticky header successfully.
Added some SS for reference.

I have reviewed 3 blocker issues and moved them accordingly, So far the MR looks good to me.
Issues reviewed:
https://www.drupal.org/project/gin/issues/3468961 🐛 Problem since rc11 with Action buttons Active
https://www.drupal.org/project/gin/issues/3488499 🐛 drupal_static data will not change each time you set it which breaks integrations like Simple Multistep Active
https://www.drupal.org/project/gin/issues/3486966 🐛 gin_form_after_build does not transfer #access to sticky actions menu Active

RTBC +1
Thanks.

Hi,
Since the issue https://www.drupal.org/project/gin/issues/3468961 🐛 Problem since rc11 with Action buttons Active , mentioned in the issue summary has been reviewed and fixed, this shall be moved as well.

I have already reviewed MR !536, it successfully fixed the button issue with Simple Multistep module, and module seems to be working fine with gin.
Hence moving this to RTBC!

Thanks.

Hi,
I was able to reproduce the issue.
Applied the MR !536.
The MR successfully fixes the issue. It adds a "Save" button on the Step 2 of the form replacing the incorrect "Next" button.

Although I noticed that the "Next" button in the Step 1 of node add form is shown inside the more actions dropdown, but the "Save" button is shown by default as a normal button. This felt inconsistent to me while adding content.
If we are okay with the button positions, this seems to be good.

Attached are the SS for reference.
Moving to RTBC for now.

Hi,
I reviewed the above mentioned issue.
Installed the gin admin theme Version: 8.x-3.x-dev, was not able to reproduce this issue.
Most probably as @saschaeggi mentioned, it got fixed with https://www.drupal.org/project/gin/issues/3486743.

@jurgenhaas, Applied the MR !536 and still was not facing this issue.

Confirming that this has been fixed.
Attaching SS for reference.

Moving to RTBC!
Thanks.

Looks great.
Attaching the updated SS for the Shortcuts.

Moving to RTBC!

Hi,
Reviewed MR !265, it removes the items from the Shortcut successfully.
All the shortcut links are working as expected.
Attached is the SS of the Shortcut after applying the changes.

Shortcuts look good, though I think we can get rid of "See all content" from the Shortcuts as the same can be achieved by clicking on "Content" which seems easier to access, as suggested above in #3.

Keeping in Needs Review for now, for inputs on above suggestion.

Thanks.

Hi @thejimbirch,
Thanks for the review.

I have removed the old config files and the same from the recipe file as well.
Also updated the recipe file to import the configs as provided by claro.

Can you please review the current/latest changes ?
Please let me know if any changes are required here.
MR Link: https://git.drupalcode.org/project/drupal_cms/-/merge_requests/246/diffs

Thanks.

Hi,
Updated the MR to fix the "Missing block" message.
Updated the recipe file to import only the necessary block configurations instead of using "*" to import all configurations.
The admin Ui looks good now.

Please review.

Hi,
Thanks for the review @thejimbirch.

I have the updated the MR accordingly.
The config files are getting imported from claro as expected, but I see a block missing error. Will debug regarding it.
Attaching SS for the same.

Hi,
Created an MR for adding the basic block configs for claro admin theme.
Added the configs in the admin_ui recipe, the site no more breaks and the sections look good.
The site branding section block looks weird with logo and site name with one below another.

Please review.

Hi,
I have created an MR for updating the content type.
@deepali sardana, I was already working on the issue and hence have created an MR. If you wish, you can review the MR provided.

Moving to Needs Review.
Please review.
Thanks.

diwakar07 → made their first commit to this issue’s fork.

Hi phenaproxima,
Thanks for reviewing my MR! I was also hesitant about this approach since it involved reusing the code.
That said, your suggestion regarding the approach is fantastic—it didn’t occur to me while I was working on it.
I’m definitely planning to experiment with other services using this method.

I have reviewed MR !223, the changes and the approach looks great and extensive as well.
It successfully removes the "Congratulations, you installed Drupal CMS!" from the page.

Moving to RTBC!

Hi,
Created an MR for removing the second status message.
Adding SS for reference.

Please review.

diwakar07 → made their first commit to this issue’s fork.

The changes look good to me.
Create links for audio file, video file, remote video and url-redirect has been removed.
Attaching the SS for reference.

Thanks.

The MR !26 looks good.
@atul_ghate Can you please resolve the merge conflicts?
Also kindly update the commit message to use the correct issue ID.

Thanks.

Thanks @timohuisman,
The MR !3 looks good to me.

Moving to RTBC!
Thanks.

Hi @timohuisman,

I reviewed MR !3. The gitlab CI file looks good but the CI tests are not running.
Please updated the file name to .gitlab-ci.yml for the CI tests to run.

Moving to NW.
Thanks.

Thanks for the work on this.
Merged.

diwakar07 → made their first commit to this issue’s fork.

Thanks for the work on this.
Merged.

diwakar07 → made their first commit to this issue’s fork.

Hi,
Made the changes to hide the message, when "Admins cannot set a password when creating or editing an account." is selected.
Please review.

Thanks & regards.

Hi,
I was able to reproduce the above mentioned bug.
I agree that hiding the message will eliminate the chances of confusion, when 'Admins cannot set a password when creating or editing an account.' is selected, as there is no option add a password when this option is selected.

Hi @joshi.rohit100,
I tried to reproduce the issue.
So, whenever adding a "<" without the respective ">", maxlength counter tried to add a closing tag(), but was skipping to close the previously opened "<" tag.
Updated the JS to add the ">" in case there is an open "<".
Now the counter looks good to me.
HTML tags are not considered as characters.

Please review.

walkingdexter → credited diwakar07 → .

Thanks for the work on this.

Please resolve merge conflicts.

Looks, Good.

Thanks, merged.

Hi,
I reviewed MR !191.
It fixes the issue and the changes look good to me.

Moving to RTBC!

Thanks,
Diwakar.

Hi,
I have reviewed MR !3.

It successfully adds an option to configure the OTP length and OTP resend duration through the config form.

Looks good to me.
Moving to RTBC!

Thanks.

@Sahana _N
Please add a row in the ServiceEndpointListBuilder.php to render the description message in the table of all service endpoints.

Thanks.

Hi,

I have reviewed MR !15.
It successfully adds a description textarea for the service endpoint, but it misses the description message in the Table of all Service Endpoints.

Attached is the SS for reference.

Hi,

Reviewed MR !17.
It successfully adds a configure link to the module Extend Page.
Attached is the SS for reference.

Looks good to me.
Moving to RTBC!

The gitlab pipeline is green.
The changes in MR !5 fixes the phpstan error.

Looks good to me.
Moving to RTBC!

Thanks.

Able to reproduce the issue in Drupal 10.3.0.
I agree with #10.

Hooks like hook_views_post_execute() or hook_views_pre_render() tend to update the #items_per_page key of the view but still renders the default pager value set in the view, rather than using the number set in custom code.
When using hook_views_query_alter(), the pager successfully gets updated.

Thanks.

Hi,
Added a column in the db to store the user delete action type.
Please review.

Thanks.

Hi,

I have reviewed MR !24, the gitlab file added follows the Gitlab template standards.
We can have separate tickets to fix the phpcs, cspell, etc. warnings.

This looks good to me.
Moving to RTBC!

Thanks.

Hi,

I reviewed MR !118 on drupal 10.3.0 and paragraphs 1.17, it successfully fixes the issue.
It adds an option in the paragraph type edit form to Enable/Disable

Save paragraph instances, even when fields are empty

, by default it is enabled.

On disabling this option, I was able to save the node by adding a blank paragraph type, and no label was rendered for the blank paragraph type added on node.
Attached are the SS for references.

This looks good to me.
Moving to RTBC!

Thanks.

Hi,
Created an MR to update the gitlab ci testing.
Followed the gitlab ci standards from Gitlab Template

Please review.
Thanks.

Diwakar07 → made their first commit to this issue’s fork.

Hi,

Reviewed the patch.
It applies successfully solving the issue mentioned.
Adds a title to the user login form.
Looks good to me.
Moving to RTBC!

Thanks.

Hi,
Reviewed MR !8.

The gitlab ci file added follows the proposed standards. We can have separate tickets for fixing the phpcs and cspell warnings.
Looks good to me.
Moving to RTBC.

Thanks.

Please review.

Hi @eelkeblok,

Configured the cspell job to break when it does not pass.
Please review.

Diwakar07 → created an issue.

Hi,

Created MR !18 that fixes the cspell issues.
Please review.

Diwakar07 → made their first commit to this issue’s fork.

Diwakar07 → made their first commit to this issue’s fork.

Hi @batigolix,

I understood the fact that you missed the crediting step.
I later got to learn that it is up to the maintainer to decide whether to credit people or not for working on an issue.

Appreciate the time you spent going back and crediting people for their work on this issue.
Hope my work on this issue was helpful, and I did not trouble much with the credit comments.

Received the credit for the issue.

Thanks,
Diwakar.

Hi,

I reviewed MR !91.
It successfully adds a new permission Edit entity sitemap settings and allows the user roles with this permission to configure sitemap settings for the specific entity on the Node itself.

But even after allowing a node to be indexed, we still need to rebuild and generate the updated sitemap from the admin interface for the changes to take affect.
If you are okay with this dependency, then this solution should be good otherwise we should stick to the base permission provided as default by the module itself.

Attached are the SS for reference.

Updated accordingly.

Thanks,
Diwakar.

Thanks for the review @Anybody.
Fixed the remaining issues, and updated the MR based on comments.

Thanks,
Diwakar.

@Anybody
Not an issue, for not being credited for the same.

I have committed the changes in the issue fork.
Please let me know if you want me create an MR for this or let it be ?

Thanks,
Diwakar.

Diwakar07 → made their first commit to this issue’s fork.

Hi @joegraduate,
Reviewed MR !10. Changes looks good to me. Fixes the ci pipeline errors.

It aligns with the latest changes in the ConfigStorageTestBase::testInvalidStorage() function in core 10.3.x.
Moving to RTBC!

Hi,
Created an MR to log the view export events based on the value of a key set in settings.php file.

For enabling the logging of the view export event, add $settings['data_export_logging'] = TRUE; in your settings.php file.
Please review.

Diwakar07 → made their first commit to this issue’s fork.

Diwakar07 → made their first commit to this issue’s fork.

Hi,

Created an MR for running gitlab ci tests for the module.
Please review.

Diwakar07 → made their first commit to this issue’s fork.

Hi,
I have fixed the remaining cspell warnings.
Please review.

Diwakar07 → made their first commit to this issue’s fork.

Hi,

I reviewed MR !7.
It successfully fixes the phpstan errors, the gitlab ci pipeline passes.
The changes are following the dependency injection.
Looks good to me.

Moving to RTBC.

Hi,

I reviewed MR !13.
PHP_CodeSniffer job is passed/green in the Gitlab CI reports.
Looks good to me.

Moving to RTBC.

Hi @kalpanajaiswal,

I have reviewed the MR !9.
The Readme file is not following the readme template standards for contributed modules.
Can we try and follow the README.md → standards ?

Moving to NW.

Hi @acbramley,

The gitlab ci file looks good to me.
We can have separate tickets to fix the issues in the pipeline.

Moving to RTBC.

Hi,

I have created an MR for fixing the cspell issues.
Please review.

Diwakar07 → created an issue.

Hi @joegraduate,

I reviewed the MR !8. It successfully fixes the phpstan issues in the pipeline.
Looks good to me.

Moving to RTBC.

Hi @Arijit Acharya,
I reviewed the MR !12.

The file follows the README.md template standards and adds a README.md file for the module replacing the README.txt.
Looks good to me.
Moving to RTBC.

All the tests in the pipeline are green.
Looks good to me.

Moving to RTBC.

Hi @charlliequadros,

I reviewed the MR !15.
Looks good to me, It adds a help page for the module.
Just a small typo in the help page description, please update theme\'s configuration to theme's configuration
Attached is the SS for reference.

Moving to NW.

Hi @joegraduate,

I reviewed the MR, looks good to me.
We can have separate issues for fixing the tests.

Moving to RTBC.

Hi @Arijit Acharya,
I have reviewed MR !12 and here are some feedback.

The Project introduction section is missing a link to the project page and the issue queue of the module.
Please refer to Readme template → and update the Project name and introduction section.
The Table of contents section is not aligned according to the README file contents.
The Maintainers section is missing from file, but added in Table of contents

Moving to NW.

Hi,
Created an MR to add the gitlab ci file.
Please review.

Diwakar07 → made their first commit to this issue’s fork.

Hi,
Created an MR to fix the cspell issues.
Please review.