🇮🇳India @Prashant.c

Amazing, happy to see this moving to the core. Well done!

lauriii → credited Prashant.c → .

@marcoliver

Thanks for your efforts on this but should not the test failures be taken as a separate issue?

This feature would be a great addition as we do not want users to fill in the registration form fields after logging in. Will try to apply the patch to the latest version of the module.

Prashant.c → created an issue.

The only way it worked is by downgrading the version of firebase/php-jwt from 6.10 to 6.5 as mentioned in this post:
https://stackoverflow.com/questions/76468099/uncaught-error-firebase-jwt...

Try to downgrade and let us know whether downgrading the library works in your case or not.

Try to composer req firebase/php-jwt:6.5.0

Hope this helps.
Thanks!

In our situation, we encountered this error after updating the site to version 9.5. We had also installed the Automated Logout module. Based on discussions here, it seemed this module might be the cause:

However, we applied a patch to the Drupal core, and currently, everything appears to be functioning well. I've attached the patch for community review. https://www.drupal.org/forum/support/installing-drupal/2023-02-13/to-log... → .

However, we patched the Drupal core and as of now seems to be working fine. Attaching the patch for community review.

Thanks

We were also facing this error after updating our Drupal site to 9.5 but not sure whether this error occurred due to the Drupal core or the Automated Logout module.

Getting same error sometimes only, sometimes working fine, not sure what is causing this to happen randomly.

We are also facing a similar issue on the Live site only which has Varnish along with Drupal's caching mechanism. For Apple login after redirection getting Login failed. Invalid OAuth2 state.

@umekikazuya Thanks for raising this and submitting the patch also, let it be tested by a few community members and then we should be good to go with it.

Cheers!

This issue was related to configuration "Key File Id", which was wrong. The description is saying Copy key file id here (prefix of the key file which is somewhat misleading, it could be improved, because sometime the person who downloaded the key file might have renamed it and in that case the person who is configuring the module might think it is the prefix of the key and not the default file name which was provided by Apple.

We can close this issue.

Thanks

@Mohd Sahzad
Thanks, you added the check for empty token but in this case the user is successfully authenticated and still the token is null.

Prashant.c → created an issue.

Prashant.c → created an issue.

@Nitin shrivastava

Thank for the review but IMHO it is too early to mark it as RTBC, let us keep it in NR until a few reviews for a couple of days may be. Looks like it is failing some tests also https://git.drupalcode.org/issue/drupal-3409379/-/jobs/599817

Thank you.

Forgot to attach the patch file in the previous comment, attaching now.

I had created this duplicate issue https://www.drupal.org/project/drupal/issues/3413413 ✨ Display machine name for the permissions Closed: duplicate , completely missed this one.

Tried to patches submitted above but were failing therefore created a new patch and a MR for the same by using mostly the changes from https://www.drupal.org/project/drupal/issues/1734534#comment-13222756

Assigning for review.

Thank you.

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

Thanks @dpi, I could not find this issue but thanks for pointing to the issue related issue.

Changing the IS to NR for community members to review and share their valuable inputs and feedback.

Thank you.

Prashant.c → created an issue.

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

Could you please add some steps to reproduce here, that would be helpful?

@qiutuo

Could you please add more details to the issue summary and Steps to reproduce as well?

Thank you

I've noticed there's been a lot of discussion on this topic, and I'd like to contribute by sharing my findings:

1. I successfully replicated this issue on version 11.x as well.
2. Could we consider making the following change in the code:

-      $query->condition('status', NodeInterface::PUBLISHED);
+      if ($this->currentUser->hasPermission('view own unpublished content')) {
+        $or = $query->orConditionGroup()
+          ->condition('status', NodeInterface::PUBLISHED)
+          ->condition('uid', $this->currentUser->id());
+        $query->condition($or);
+      }
+      else {
+        $query->condition('status', NodeInterface::PUBLISHED);
+      }

It seems to yield the expected results, displaying unpublished content for which the user is the author, while the default remains Published only.

I would appreciate your thoughts and feedback on this.

Thank you.

@omkar.podey great work!

I have following observations:

1. When the modal window opened by default the "Plain text" field is highlighted. I think it should not be highlighted by default because we are going to the step 2 after clicking the field type and in this case we have to click "Plain text" field once again.

2. In the modal window Step2 the field type which was selected in Step1 should be displayed as modal window currently it is displaying the title as"Add field" and sometime when there is not much info on the step2 for example in case of adding a Link field we need this title as the field type. Therefore currently it is not clear to user which field was selected in Step1 and it would be really helpful if the field type can be displayed as the Modal window title.

3. The submit buttons "Continue" and "Back" should be there on the left side instead of the right side as those are there in the existing flow.

Thank you.

Can we hide this MR https://git.drupalcode.org/project/drupal/-/merge_requests/5860?

1. The trim_length property is of integer type

trim_length:
      type: integer
      label: 'Trim link text length'

therefore default value is required in integer not string.

2. Assigned dafault value to

'trim_length' => 0

in LinkFormatter.php and LinkSeparateFormatter.php

3. Changed the '#min' => 1 to '#min' => 0 .

4. It solves the truncate issue and we can assign the 0 from the UI as well in the manage fields.

5. Raising the MR and attaching the patch as well for 11.x

Thanks

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

@cilefen Yes, at-least I got this error on clean 11.x install.

I was able to reproduce the issue with the steps mentioned in the issue summary and the code changes fixes the issue and it working as expected for the users with permission to Create new content block.

Can we add tests for this in this file /core/modules/block_content/tests/src/Functional/BlockContentCreationTest.php itself?

Thanks

Applied the patch provided patch → , added Dependency injection for the services, raised MR as well.

Thank you

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

This patch is not applying on 10.1.x or 11.x. Request you to re-submit the patch.

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

Added the null coalescing operator to occurrences of strnatcasecmp()wherever it was applicable.

I was not able to apply the patches provided in 11.x therefore submitting the patch file as well.
Changes needs to be reviewed.

Thank you.

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

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

Submitting Patch for 11.x and raising the MR as well.

Thank you.

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

Thanks for raising and the fix @Gauravvvv. Yes, I am also able to reproduce this issue, however this JS error appears on every content type not specifically on "Basic page" only.

Put some comments in the MR.

Thank you.

Used the same pattern which is used to develop other components/sections to decouple the "Help" link from the footer. Here is the MR https://git.drupalcode.org/issue/navigation-3411099/-/merge_requests/2.

Kindly review, thanks.

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

I think this issue will have dependency on https://www.drupal.org/project/navigation/issues/3412119 📌 Restrict configuration of the Navigation bar footer items Fixed

I'm thinking whether it's okay to use CSS here to hide the label and select list for regions.

If we just want to make one link that always takes the user or admin to the configuration page "/admin/config/user-interface/navigation-section" I don't think it's needed. The admin can already go to this page using the module's configure link or by going to "/admin/config" For me, having this link doesn't make sense. I'd like to hear what others think about it.

Thank you.

@m4olivei

As soon as Iam changing the form element from "select" to "value" or something else the tabledrag stops working, not able to drop items and JS error The navigation section cannot be placed in this region.

Any ideas?

Raised MR https://git.drupalcode.org/issue/navigation-3411099/-/merge_requests/1 and created the merge request against the https://git.drupalcode.org/issue/navigation-3411099/-/tree/3411099-creat....

Kindly review.

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

Included code to achieve the following functionalities:

1. Show the "Download" button specifically on the single export page at "admin/config/development/configuration/single/export".

2. Upon clicking the "Download" button, a YAML file with the configuration name will be downloaded to your local system.

Requesting code and usability reviews, as well as any feedback. Appreciate your input. Thank you.

We're currently addressing the same issue https://www.drupal.org/project/drupal/issues/3399927 ✨ Link to Config Item from configuration synchronize page Needs work for the mentioned feature in an existing thread, which serves as a suitable platform for collaboration. In my opinion, we can consider closing this particular one.

Not sure about the IS but keeping it "Postponed" until the other one is closed.
Please don't hesitate to reopen if you hold differing opinions.
Thank you.

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

1. Introducing a new link labeled "Import this configuration."
2. Clicking on the link will redirect the user to "/admin/config/development/configuration/single/import/[config_type]/[config_name]."
3. The "Paste your configuration here" textarea will be automatically populated with the original configurations.
4. Press the "Import" button to initiate the import of the default configurations.

However, there are still pending tasks, including code review, code cleanup, and testing.

1. The concern mentioned in Point 2 on https://www.drupal.org/project/navigation/issues/3411099#comment-15378332 📌 Create an administration UI for managing Navigation Sections Needs work has been resolved.
2. The remaining two points pertain to frontend issues.
3. Should we consider treating the conversion of jQuery to JS as an independent task for the entire module?
4. Currently, it seems that decisions regarding "Reset to default" and "Contextual links" are the only outstanding matters.

Overall, looks good to me.

1. I concur that there should be a "Reset to Default" button available on /admin/structure/navigation-section for resetting the navigation section. This feature is essential in case users or admins inadvertently modify the settings and wish to revert to the default state. Currently, this option is unavailable, and having it would be a valuable addition.

2. Despite updating the section title in the backend, the displayed title remains unchanged. Refer to the image below for clarification:

3. There is an issue with the scrolling functionality when the "Administration" section is positioned in the footer. The navigation bar does not scroll due to an overlap problem, and no scrollbar appears. Please see the image below for a visual representation:

Changing IS to NW.

@m4olivei, excellent job!

Thanks, @m4olivei! I didn't notice that you had already assigned it. Nevertheless, I'm looking forward to exploring the features, especially the "Place navigation section." Cheers!

Updated the docblock sentences in the hook documentation for clarity:

 /**
- * Change the view mode of a particular entity type that is being displayed.
+ * Change the view mode of a specific entity type currently being displayed.
  *
  * @param string $view_mode
- *   The view_mode that is to be used to display the entity.
+ *   The view_mode currently displaying the entity.
  * @param \Drupal\Core\Entity\EntityInterface $entity
  *   The entity that is being viewed.
  *
  * @ingroup entity_crud
  */

Additionally, incorporated review comments.
Thank you.

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

1. I successfully replicated the issue using the steps outlined in the "Steps to reproduce." In essence, the "Parent terms" section under "Relations" is displaying term names for all languages in the term list.
2. The patch/MR addressing this problem ensures that only terms relevant to the respective language are listed in the "Parent terms" section.
3. The code has been refactored and requires both code review and thorough testing, including manual and automated tests.

Thank you.

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

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

@jurgenhaas As per https://www.drupal.org/project/drupal/issues/2761273#comment-14937174and IMO also we should keep the scope of this issue to provide to provide exposed filter values available as tokens only and other requests follow up issues could be created.

Thanks!

Could not find the link to open the entity page therefore added code for the following:
1. Added the collection route for the entity
2. Added links file

Will keep testing.

Thank you.

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

During my local testing, I observed that when adding a new field and being redirected to the field form after selecting the field type, the URL includes a query parameter, such as ?entity_type=node , specifically for the content type. This query parameter is actually necessary ?

Thank you.

Incorporated code from version 1.x. @finnsky, could you kindly review for any necessary modifications and confirm the status of this issue, whether it remains valid or not?

Thanks

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

Implemented a label for the aside section, but it is recommended to review the text. Additionally, consider finalizing labels for the nav sections with "content" having id="menu-builder" and "footer" having id="menu-footer".

Your review is appreciated. Thank you.

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

Attempted to rebase with version 1.x, but encountered numerous conflicts in CSS files. Deciding to leave it for a front-end developer to address.

On desktop, the menu items appear to be displayed and functioning correctly with JavaScript disabled. However, identified an issue where menu items are not shown as active.

Thanks.

In my opinion, it's acceptable for multiple menu items to share the same link and display them as active. This is because, if we prioritize these links or don't highlight others as active, users and administrators may not be aware that there are multiple links with the same URL. Additionally, there could be intentional reasons for having multiple links with the same URL. As a result, this should not pose any harm, but further discussion or feedback is encouraged.

Thank you.

@m4olivei

Awesome! Eagerly anticipating this; I hope to make a meaningful contribution here too. :)

Appreciate it.

@m4olivei

Thanks again for a review. Addressed the review comments.

1. I agree font family and size etc. needs to be fixed in the Top bar.
2. Queries in https://www.drupal.org/project/navigation/issues/3402046#comment-15372914 ✨ Create the Top Bar Needs review still needs response from @ckrina.

Keeping it in NW for above.

Thank you.

I attempted to create a change record https://www.drupal.org/node/3411040 → (Draft) as mentioned in #17 📌 Optimize user logins by avoiding duplicate entity queries Needs work . It may require a lot of changes.

Kindly review, thankyou.

@cilefen

Appreciate the provided links to related issues. The issue https://www.drupal.org/project/drupal/issues/3201567 → seems to cover one of the requirements. Another requirement is to include an option like "Export," which would directly export changes to the codebase in the configuration folder if feasible.

However, we may want to wait until other issues are resolved. As of now, I'll mark this as "Postponed."

Thank you.

Issue already present for the same https://www.drupal.org/project/adminimal_admin_toolbar/issues/3285987 📌 Automated Drupal 10 compatibility fixes Fixed

But how you were able to install standard profile with existing configs, it only works with minimal as far as I tried
https://www.drupal.org/node/2897299 → .

Therefore for me in steps to reproduce it is failing on Step 5.

Thank you.

As per the comments in https://www.drupal.org/project/config_split/issues/3362191#comment-15074998 📌 Remove Drush 8 integration Needs review , removing the config_split.drush.inc file. Changing status to NR.

Thanks

Prashant.c → changed the visibility of the branch 3362191-drupal-10-compatibility to active.

Prashant.c → changed the visibility of the branch 3362191-remove-drush-8-compatibility to hidden.

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

@mikko123

When you hide a field on the "Manage form display" page, it will be excluded from the create/edit forms of the corresponding entity. This action is not associated with "Manage display."

The rendering of the field is contingent on the chosen field formatter, and users have the option to create their own field formatter.

For those who wish to modify the output without creating a new formatter, the hook_preprocess_HOOK function can be utilized. More information can be found at: hook_preprocess_HOOK documentation

For instance, if the HOOK is "comment," and "custom" represents the custom module's name:

/**
 * Implements hook_preprocess_HOOK().
 */
function custom_preprocess_comment(&$variables) {
  unset($variables["title"]);
}

This should address the current issue. If there are any concerns or if something has been overlooked, please feel free to reopen the issue.

Thank you!

1. There is a comment in the MR that this has an out of scope change

DONE

2. The test should also test when there is a label.
   

   It is there https://git.drupalcode.org/project/drupal/-/merge_requests/5550/diffs#dc...

3. The change record should explain what the return value will be when there is a label and when there is not. Currently, it is a misleading.
   

   Still pending

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

Took the changes from #16 🐛 User data service does not take cache tag invalidation into account Needs work to create a MR.

It seems still a lot of work is left to be done in this. Keeping in NW.

Thanks

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

Attempting to make a contribution here, I've modified aspects concerning strict types in both function parameters and return values. After local testing, I can confirm that the login functionality is functioning as expected following these adjustments. Updating the status to NR.

Thanks