Part of #1921152: META: Start providing tour tips for other core modules.

Problem/Motivation

Write tour integration for People admin page

Proposed resolution

Create tour yml files for required admin screens in admin/people.

Remaining tasks

Create patch for tour integration for primary People page

User interface changes

New tours

API changes

None

Technical pointers when creating tour tips

See: https://drupal.org/node/1921152#tour-tips-tech-note for tech notes on making tour tips.

CommentFileSizeAuthor
#94 Screenshot 2022-02-17 at 9.50.11 AM.png148.88 KBjoshua1234511
#94 Screenshot 2022-02-17 at 9.49.47 AM.png95.89 KBjoshua1234511
#94 interdiff_90_94.txt546 bytesjoshua1234511
#94 2040845_94.patch4.14 KBjoshua1234511
#90 interdiff_84_90.txt2.67 KBjoshua1234511
#90 2040845_90.patch4.14 KBjoshua1234511
#87 2040845.mov1.73 MBgargsuchi
#85 people-tour.gif293.11 KBthejimbirch
#84 interdiff_81-84.txt1.17 KBvsujeetkumar
#84 2040845_84.patch3.84 KBvsujeetkumar
#81 interdiff_80-81.txt792 bytesvsujeetkumar
#81 2040845_81.patch3.9 KBvsujeetkumar
#80 2040845_80.patch3.91 KBvsujeetkumar
#78 drupal-admin-people-tour4.png255.23 KBKristen Pol
#78 drupal-admin-people-tour3.png281.26 KBKristen Pol
#78 drupal-admin-people-tour2.png244.85 KBKristen Pol
#78 drupal-admin-people-tour1.png256.57 KBKristen Pol
#75 interdiff_73-75.txt1.35 KBvsujeetkumar
#75 2040845_75.patch3.91 KBvsujeetkumar
#73 interdiff_70-73.txt792 bytesvsujeetkumar
#73 2040845_73.patch2.7 KBvsujeetkumar
#72 Screenshot from 2021-04-18 10-06-11.png32.27 KBbarone
#70 interdiff_68-70.txt2.64 KBvsujeetkumar
#70 2040845_70.patch3.88 KBvsujeetkumar
#68 interdiff_64-68.txt3.55 KBvsujeetkumar
#68 2040845_68.patch3.64 KBvsujeetkumar
#64 interdiff_59-64.txt900 bytesvacho
#64 tour-people-2040845-64.patch3.56 KBvacho
#59 codesniffer_fixes.patch934 bytesprashantgajare
#59 tour-people-2040845-59.patch2.33 KBprashantgajare
#57 tour-people-2040845-57.patch2.44 KBclemens.tolboom
#40 tour-people-2040845-40.patch2.39 KBhussainweb
#40 interdiff.txt583 byteshussainweb
#37 interdiff-2040845-35-37.txt1.11 KBjorgegc
#37 tour-people-2040845-37.patch2.39 KBjorgegc
#35 tour-people-2040845-35.patch2.79 KBjorgegc
#26 interdiff.txt1.8 KBpameeela
#26 tour-people-2040845-26.patch2.58 KBpameeela
#25 tour-people-2040845-25.interdiff.txt378 bytesnick_schuch
#25 tour-people-2040845-25.patch2.22 KBnick_schuch
#22 tour-people-2040845-22.patch2.22 KBnick_schuch
#19 tour-people-2040845-17.interdiff.txt389 bytesnick_schuch
#19 tour-people-2040845-17.patch2.24 KBnick_schuch
#17 tour-people-2040845-17.patch2.24 KBnick_schuch
#17 tour-people-2040845-17.interdiff.txt389 bytesnick_schuch
#14 tour-people-2040845-14.patch2.23 KBnick_schuch
#14 tour-people-2040845-14.interdiff.txt3.09 KBnick_schuch
#10 tour-people-2040845-10.patch2.66 KBfloydm
#10 interdiff-2040845-8-10.txt3.13 KBfloydm
#8 tour-people-2040845-8.patch5.78 KBfloydm
#8 interdiff-2040845-5-8.txt2.37 KBfloydm
#5 tour-people-2040845-5.patch3.93 KBfloydm
#5 interdiff-2040845b-5.txt2.03 KBfloydm
#2 tour-people-2040845b.patch2.38 KBlisarex
#1 tour-people-2040845.patch1.02 KBlisarex

Issue fork drupal-2040845

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lisarex’s picture

FileSize
1.02 KB

Content written and edited by colleagues :)

lisarex’s picture

Status: Active » Needs review
FileSize
2.38 KB

Ignore previous. This patch contains two yml files

larowlan’s picture

Re the image, I don't think 'any site visitor' is true, I think 'approved users' would be more correct, as it's permission based.

nick_schuch’s picture

Issue tags: +Needs tests

Tour needs tests since we now have #2028535

floydm’s picture

Updated the patch to add UserAdminTourTest.php and fix a couple of whitespace issues in the yml file.

One of the tests fails because it can't find an element with class .action-links, which the second tip is attached to. However I can see such an item in the verbose testing output:

<nav class="action-links">
<li>
<a class="button button-action" href="/d8/admin/people/create">Add user</a>
</li>
</nav>

If I attach the second tour tip to class "button-action" the test passes. Does SimpleTest not handle the html 5 nav element?

Status: Needs review » Needs work

The last submitted patch, tour-people-2040845-5.patch, failed testing.

lisarex’s picture

Assigned: lisarex » Unassigned

unassigning myself

floydm’s picture

Updated patch that adds the test for the /user/*/edit tour. It also modifies the user admin tour test to no longer test for nav class="action-links", which I mentioned in comment #5 I could not get to pass and which I suspect has to do with it being an html5 nav element. Only a guess though.

floydm’s picture

Status: Needs work » Needs review

Changing status to needs review.

floydm’s picture

Just noticed issue #2044399: Write tour integration for user edit page exists for the /user/*/edit tour and that that tour appears more complete, so I am rerolling this patch to remove the tour and test for /user/*/edit.

nielsonm’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Works for me. Marking this as RTBC.

batigolix’s picture

Status: Reviewed & tested by the community » Needs review

The code may work but the content & wording needs a proper review first

nielsonm’s picture

Assigned: Unassigned » nielsonm
Status: Needs review » Needs work
Issue tags: +Tour Content Review

Needs content review and new router system based update

nick_schuch’s picture

This patch updates the path to a route and updates the test coverage.

Status: Needs review » Needs work

The last submitted patch, 14: tour-people-2040845-14.patch, failed testing.

The last submitted patch, 14: tour-people-2040845-14.patch, failed testing.

nick_schuch’s picture

FileSize
389 bytes
2.24 KB

Fixes yml format issue.

nick_schuch’s picture

Status: Needs work » Needs review
nick_schuch’s picture

FileSize
2.24 KB
389 bytes

Reuploading for tests to run.

Status: Needs review » Needs work

The last submitted patch, 19: tour-people-2040845-17.patch, failed testing.

The last submitted patch, 17: tour-people-2040845-17.patch, failed testing.

nick_schuch’s picture

Status: Needs work » Needs review
FileSize
2.22 KB

Status: Needs review » Needs work

The last submitted patch, 22: tour-people-2040845-22.patch, failed testing.

nielsonm’s picture

Assigned: nielsonm » Unassigned
nick_schuch’s picture

Status: Needs work » Needs review
FileSize
2.22 KB
378 bytes

Time for some green!

pameeela’s picture

Updates content to better align with guidelines.

sun’s picture

This comment raises architectural design concerns and is not meant to discredit the work that went into this particular issue and patch.

+    label: 'Managing users'
+    body: 'View and edit user accounts.'
...
+    label: 'Add a user'
+    body: 'Create a new user account.'
...
+    label: 'Search for users'
+    body: 'Find users by applying filters.'
...
+    label: 'Update accounts'
+    body: 'Select one or more users via the checkboxes to apply changes to the accounts.'
...
+    label: 'Edit user account'
+    body: 'Make changes to one user account.'

Hrm. — Do we really want to explain every possible UI interaction in tour tips?

If we do, what kind of "tip" or explanation is "Create a new user account." meant to give that wouldn't be obvious by the UI already?

I was under the assumption we introduced Tour tips to explain non-obvious things in the user interface only?

→ If we need to provide hints for the most fundamental/basic operations, then our UI/UX must be seriously flawed? (which I don't think it is)

I'm afraid, this makes little sense to me. These kind of Tour tips will only make people avoid Tour module whenever possible.

Introducing any kind of such "tips" presents a major disrespect and discredit for the hard work of Drupal's Usability and Design teams, because the mere fact of pointing out the most basic user interface operations via Tour tips would mean that the entire user interface design is a utter #FAIL.

Or to put that in reverse, because it might not be obvious:

The user interface design is solely responsible for making obvious things obvious. User interface design enables end-users to use Drupal intuitively. A well-designed user interface needs absolutely zero explanation; every human immediately understands how to use it. That is the one and only goal of user interface design.

This patch (and likely others) introduces tips for the most obvious operations in the user interface. These operations should not need any kind of explanation in the first place. IF they do, then the solution is NOT to explain to the user how to use the interface. — The only acceptable solution is to fix the user interface design.

In short, if there is a problem with any of these basic operations, then we need to fix the UI. Tour tips should be reserved for non-obvious interactions.

A non-obvious interaction would be to point out that it is possible to add fields to users. Or to add view modes. Or that you can change the administrative user listing/view to customize it to your needs. Or that you can leverage the "Reset password" functionality to temporarily grant someone else access to a user account without sharing the password. — All of that is not obvious, and cannot be solved by user interface design.

batigolix’s picture

Some background discussion about the role of tours can be found in #1921152: META: Start providing tour tips for other core modules.

pameeela’s picture

sun, I could not possibly agree more. I think there has been some motivation to get more tours in and that has resulted in trying to create ones for a lot of admin pages that don't really need them. Or maybe this page should have one but as you said, it should point out the less obvious things.

There should probably be some more discussion about this sooner than later, to avoid unnecessary work.

rodrigoaguilera’s picture

Status: Needs review » Needs work

The routes section is separated in two lines.

the test need annotation to be detected as such instead of getInfo() and at least a setUp function and some actual tests.
Is as simple as the following example

  /**
   * {@inheritdoc}
   */
  protected function setUp() {
    parent::setUp();
    $this->adminUser = $this->drupalCreateUser(array('administer content translation', 'access tour'));
    $this->drupalLogin($this->adminUser);
  }

  /**
   * Tests Content translation tour tip availability.
   */
  public function testContentTranslationTour() {
    $this->drupalGet('admin/config/regional/content-language');
    $this->assertTourTips();
  }
rodrigoaguilera’s picture

Issue tags: +Novice

marking it as novice since is a task that can teach a lot about the drupal system

jorgegc’s picture

I am happy to work on this one guys.

jorgegc’s picture

Status: Needs work » Needs review
FileSize
2.79 KB

Uploading patch for review. Please let me know if there is anything else that we need to test. I've basically looked at what we are doing in Forum and replicated it.

benjy’s picture

Status: Needs review » Needs work
  1. +++ b/core/modules/user/src/Tests/UserAdminTourTest.php
    @@ -0,0 +1,61 @@
    +  /**
    +   * {@inheritdoc}
    +   */
    +  protected function setUp() {
    +    parent::setUp();
    +    $this->adminUser = $this->drupalCreateUser(array('administer users', 'access tour'));
    +    $this->drupalLogin($this->adminUser);
    +  }
    

    Remove this, the user should be created in the parent using the permissions from $permissions.

  2. +++ b/core/modules/user/src/Tests/UserAdminTourTest.php
    @@ -0,0 +1,61 @@
    +  /**
    +   * Tests user list tour tip availability.
    +   */
    +  public function testUserListTour() {
    +    $this->drupalGet('admin/people');
    +    $this->assertTourTips();
    +  }
    

    This isn't needed, see TourTestBasic::testTips().

jorgegc’s picture

Status: Needs work » Needs review
FileSize
2.39 KB
1.11 KB

Changes as per comments in #36.

benjy’s picture

Status: Needs review » Reviewed & tested by the community

Looks great. RTBC if we're green.

cilefen’s picture

+++ b/core/modules/user/src/Tests/UserAdminTourTest.php
@@ -0,0 +1,42 @@
+   * The permissions required for a logged in user to test tour tips.

This is a total nit, but logged-in is a compound adjective and it should be hyphenated. You will find the wrong way in the same file as the right way in core, like ForumTest.

hussainweb’s picture

Issue tags: +DrupalSouth
FileSize
583 bytes
2.39 KB

I see http://english.stackexchange.com/a/11002 which also points to the same suggestion. Changing it. :)

I think this can still be RTBC.

jorgegc’s picture

Thanks @hussainweb :-)

webchick’s picture

Status: Reviewed & tested by the community » Postponed

Thanks a lot for your work on this issue! I reviewed it and it seems to work well.

I do have some concerns, however:

#1: The page in question is a View. In this respect it's identical to admin/content, admin/comments, etc. but those pages do not (yet) have Tours. Therefore, committing this patch would introduce an inconsistency with other, similar admin pages.

#2: I also share some of sun/pameela's concerns from #27/#29. We need to come to agreement on guidelines of when we do/do not use Tours for certain admin interactions.

Therefore, very sorry to do this, but I think we need to mark this postponed on figuring out such guidelines up in the parent issue at #1921152: META: Start providing tour tips for other core modules..

Since you've done great work on this and had the opportunity to play with Tour module, please chime in there with your thoughts!

mgifford’s picture

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

DrupalMattS’s picture

Version: 8.5.x-dev » 8.6.x-dev
Assigned: Unassigned » DrupalMattS
Status: Postponed » Active

#Nashville2018 I am opening this issue in the DrupalCon Nashville Tour Mentored sprint

DrupalMattS’s picture

Based on previous comment from Comment webchick I will mark back as postponed

DrupalMattS’s picture

Assigned: DrupalMattS » Unassigned
Status: Active » Postponed
grgcrlsn321’s picture

Looking into this.

grgcrlsn321’s picture

Status: Postponed » Reviewed & tested by the community

Applied patch and the tour for the People (admin/people) seems to already be working good. The tour goes through each button used on the page, unless there is need for more details I would consider this patch ready for commit.

grgcrlsn321’s picture

Status: Reviewed & tested by the community » Needs review

Setting this back to needs a review. The css is a little bit off and their isn't much description. The buttons pointed out seem pretty self explanatory. Maybe this tour is quite needed seeing that these steps our used throughout drupal admin pages.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

John Cook’s picture

Status: Needs review » Needs work

I've tested the patch from comment 40 but the "Tour" button did not appear after a new install of Drupal.

  • --- /dev/null
    +++ b/core/modules/user/config/install/tour.tour.people.yml
    

    This should be in the optional config folder as the Tour module may not be enabled.

  • In the tour.tour.people.yml file there needs to be a dependencies section:
    dependencies:
      module:
        - user
    

    This allowed the tour to be accessible.

clemens.tolboom’s picture

Status: Needs work » Needs review
FileSize
2.44 KB

I moved the tour to config/optional similar to views_ui and added dependencies as suggested in #56

've tested the patch from comment 40 but the "Tour" button did not appear after a new install of Drupal.

I've seen that a lot using Tour UI so that is not a bug for this issue. It may be worth to submit a general issue but I guess `drush cach-rebuild` fixes it.

Status: Needs review » Needs work

The last submitted patch, 57: tour-people-2040845-57.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

prashantgajare’s picture

prashantgajare’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 59: codesniffer_fixes.patch, failed testing. View results

prashantgajare’s picture

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

vacho’s picture

Solving test problems over patch #59

volkswagenchick’s picture

Issue tags: +drupalnorth2019

Tagging for DrupalNorth 2019

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

vsujeetkumar’s picture

Status: Needs work » Needs review
FileSize
3.64 KB
3.55 KB

More Test Fixes. Please review.

Status: Needs review » Needs work

The last submitted patch, 68: 2040845_68.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

vsujeetkumar’s picture

Fixed Tests, Please review.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

barone’s picture

The patch failed to apply. Drupal 9.2-x.dev

Patch failed

vsujeetkumar’s picture

Re-roll patch created for 9.2.x.

Status: Needs review » Needs work

The last submitted patch, 73: 2040845_73.patch, failed testing. View results

vsujeetkumar’s picture

Status: Needs work » Needs review
FileSize
3.91 KB
1.35 KB

Fixed the fail test.

clemens.tolboom’s picture

I always wonder why patch contributors are removed from the list.

In this case
- prashantgajare
- vacho
- clemens.tolboom

AFAIK there is no tracking or reasoning for that.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Kristen Pol’s picture

Thanks for the update.

1. Patch still applies cleanly to 9.3.

2. Manually tested as follows:

  • Installed 9.3 with standard profile
  • Logged in as user 1 (admin)
  • Made sure tour module was enabled
  • Went to /admin/people
  • Clicked on Tour link in upper right
  • Went through the tour (see screenshots)

Kristen Pol’s picture

Status: Needs review » Needs work

Here's some feedback based on the above and reviewing the patch:

  1. +++ b/core/modules/user/config/optional/tour.tour.people.yml
    @@ -0,0 +1,50 @@
    +  people-add:
    +    id: people-add
    +    plugin: text
    +    label: 'Add a user'
    +    body: 'Create a new user account.'
    +    weight: 2
    +    attributes:
    +      data-class: button-action
    

    This step isn't triggering because the button-action class has been changed to button--action. Perhaps use button--primary to be more precise for this page?

  2. +++ b/core/modules/user/config/optional/tour.tour.people.yml
    @@ -0,0 +1,50 @@
    +    body: 'Select one or more users via the checkboxes to apply changes to the accounts.'
    

    Nitpick: Consider simplifying to something like:

    Apply changes after selecting one or more accounts via the checkboxes.
    
  3. +++ b/core/modules/user/tests/src/Functional/UserAdminTourTest.php
    @@ -0,0 +1,51 @@
    +    $this->assertTourTips();
    

    Not sure why this doesn't this catch the issue of step 2 not showing up.

vsujeetkumar’s picture

Status: Needs work » Needs review
FileSize
3.91 KB

Re-roll patch given for 9.3.x.

vsujeetkumar’s picture

Addressed #79.1 and 79.2, Please have a look.

The last submitted patch, 80: 2040845_80.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 81: 2040845_81.patch, failed testing. View results

vsujeetkumar’s picture

Status: Needs work » Needs review
FileSize
3.84 KB
1.17 KB

Fixed the fail test, It is basically related to some deprecated property in config schema.

thejimbirch’s picture

FileSize
293.11 KB

The patch applies. I did get fuzz message that I have never seen before though:

patch -p1 < 2040845_84.patch
patching file core/modules/migrate_drupal_ui/tests/src/Functional/d6/Upgrade6Test.php
Hunk #1 succeeded at 99 with fuzz 2.
patching file core/modules/migrate_drupal_ui/tests/src/Functional/d7/Upgrade7Test.php
Hunk #1 succeeded at 101 with fuzz 2.
patching file core/modules/user/config/optional/tour.tour.people.yml
patching file core/modules/user/tests/src/Functional/UserAdminTourTest.php

To test, I still had to import the config manually, since the user module was already enabled. Adding a tour to core will only show up for new users. Should there be an update hook to add the tour to existing sites?

Leaving this as Needs Review to answer that question, but the current patch works as expected.

Here's a screen recording of the tour.

Screen recording of the tour.

gargsuchi made their first commit to this issue’s fork.

gargsuchi’s picture

FileSize
1.73 MB

Manually tested as follows:

  1. Installed 9.3 without a profile
  2. Logged in as user 1 (admin)
  3. Made sure tour module was enabled
  4. Went to /admin/people
  5. Clicked on Tour link in upper right
  6. Went through the tour (see video attached)

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Kristen Pol’s picture

Status: Needs review » Needs work
Issue tags: -Novice

Thanks for the updated patch and the testing. Removing the novice tag as I don't think it applies anymore. From #85:

To test, I still had to import the config manually, since the user module was already enabled. Adding a tour to core will only show up for new users. Should there be an update hook to add the tour to existing sites?

It should show up for existing sites, so back to needs work for that at least. Additionally, some minor nitpicks below:

  1. +++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d6/Upgrade6Test.php
    @@ -99,7 +99,7 @@ protected function getEntityCounts() {
    -      'tour' => 6,
    +      'tour' => 7,
    

    Unrelated to issue so shouldn't be in patch.

  2. +++ b/core/modules/migrate_drupal_ui/tests/src/Functional/d7/Upgrade7Test.php
    @@ -101,7 +101,7 @@ protected function getEntityCounts() {
    -      'tour' => 6,
    +      'tour' => 7,
    

    Same as above.

  3. +++ b/core/modules/user/tests/src/Functional/UserAdminTourTest.php
    @@ -0,0 +1,51 @@
    +  protected static $modules = ['block', 'user', 'tour', 'views'];
    

    Extreme Nitpick: Not sure if it's customary to make these alphabetical.

joshua1234511’s picture

Status: Needs work » Needs review
FileSize
4.14 KB
2.67 KB

Updated the patch with a update_hook to make it work with existing sites and not having to reinstall the module.
Included the Nitpick/ code review suggestions from #89

Status: Needs review » Needs work

The last submitted patch, 90: 2040845_90.patch, failed testing. View results

Kristen Pol’s picture

Thanks for the update @joshua1234511.

Is this code similar to other code in core? I was hoping to compare it, if so.

+++ b/core/modules/user/user.install
@@ -134,3 +135,29 @@ function user_update_9301(&$sandbox) {
+ * Import tour people user configurations.

Nitpick: Reword to something like: Import people tour configuration.

joshua1234511’s picture

Would need to update this as it has missed the window for 9.3. Therefor change the name to system_update_9401()

joshua1234511’s picture

Status: Needs work » Needs review
FileSize
4.14 KB
546 bytes
95.89 KB
148.88 KB

Updated the patch with the nitpick and hook update support for v9.4.x

The code for hook update import YML referenced form
web/core/modules/config/src/Form/ConfigSingleImportForm.php
web/core/lib/Drupal/Core/Config/ConfigInstaller.php
Core Code
Core code

Status: Needs review » Needs work

The last submitted patch, 94: 2040845_94.patch, failed testing. View results

clemens.tolboom’s picture

The

+function user_update_9302() {

seems unique (or we have still to few tours added to core) in (re)loading the tour.

I vaguely remember a discussion about updates should not update tours. Cannot find it :-/ Maybe it was Tour UI #3214357: Will changes from (core 9.2+) Tour hurt? ?

thejimbirch’s picture

According to the comment in #89,

It should show up for existing sites, so back to needs work for that at least.
clemens.tolboom’s picture

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

Status: Needs work » Postponed
Issue tags: -Tour

This extension is being deprecated, see #3336033: [Meta] Tasks to deprecate Tour module. It will be removed from core and moved to a contrib project, #3376099: [11.x] [Meta] Tasks to remove Tour.

This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.