jhedstrom / drupalextension

An integration layer between Behat, Mink Extension, and Drupal.
GNU General Public License v2.0
209 stars 192 forks source link

Add a helper for the getting of the login submit element #631

Closed eiriksm closed 1 year ago

eiriksm commented 2 years ago

OK, so I have a use case it's kind of convoluted to achieve at the moment.

The login method will fill in fields and submit the form to log in. If for some reason you have changed the default markup, you can override the way this is selected by providing a value for the "drupal text" log_in.

This is convenient if you use English, and only that. That way you could probably add "Log in" and be done with it. If you want to accommodate that the site can theoretically at different stages have this part translated, or not, then using the string for the value of the submit is not so convenient. Luckily, we use the method findButton which does this:

    /**
     * Finds button (input[type=submit|image|button|reset], button) with specified locator.
     *
     * @param string $locator button id, value or alt

OK, so I can use the ID or the value. Since value is out of the question, let's try ID. So I change it to "#edit-submit", which is the ID of the submit button.

Well, guess what. Drupal can change that based on the order of cache being saved. So theoretically, you can have that as the id of the button at one point, but then something re-renders, you visit a page with a search form, then visit the login form while the search form is there. Now the search form has that as its id on the same page. Inconvenient.

But OK, let's just override how we submit it. I would like to do that. What I now would have to do is override the login method of the authentication manager, and copy paste all the code in there. What if I could just override one method of the manager instead, and use a different selector. For example, I could swap it out to just use

$submit = $element->find('css', '.user-login-form [data-drupal-selector="edit-submit"]');

That would be possible if we added a protected method for getting the element. A much safer and maintainable override.

I would also mention that it would be more convenient to have it as a "drupal selector", so if that is more interesting, I could create another PR for that?

jhedstrom commented 1 year ago

I like this idea, but I wonder if we are substituting one issue for another? This method relies on the form having that specific class, which in theory is theme-dependent. Could we possible use this to fallback in case the text method currently in place fails?

eiriksm commented 1 year ago

Very valid point! I'll see if I can get that in there

eiriksm commented 1 year ago

Wait, I think you are misunderstanding.

The PR contains the exact same code as earlier, it just makes it possible for me to override it on a per-project basis. I feel that's a good first step? So this PR is totally backwards compatible and works exactly like the current version of this package. Feels safest to me?

jhedstrom commented 1 year ago

The PR contains the exact same code as earlier, it just makes it possible for me to override it on a per-project basis.

Ah, yes, I did misread the change. This looks good. Thanks!