fathershawn → created an issue.
Nice use of once
property to verify the behavior had done its work +1!
With 10.2-alpha1 out should we move the tag to 10.3 and release all the functionality together?
fathershawn → created an issue.
New test passes.
Missed bringing over the Twig function
All tests green
All tests green
It might not be, but if we refactor BigPipe to use HTMX we should return HTML - probably with the placeholder as the main content, and I wondered if that might intersect with this use case??
fathershawn → created an issue.
fathershawn → created an issue.
I'm so glad to be working with collaborators who have a historical perspective on the code! This is an enhancement not a requirement that will give a bit of performance boost. I'm going to postpone it and move it down the task list before BigPipe.
In the module, this simplified HTML response is a route option and scanning the linked history that may be a good thing. Let's think about routes we might build to serve requests from HTMX and pick up this work in a while. Ideas that I have for routes are:
- the one I have in the module
/htmx/{entityType}/{entity}/{viewMode}
- one to render placeholders as the main content
RE: #16 - yes we have that covered in 📌 Process attachments (CSS/JS) for HTMX responses and add drupal asset libraries Active
All other unresolved MR comments are from the previous MR which we have closed.
I can't mark the MR comment resolved but I removed the version requirement from the info.yml file in the test module.
@longwave Okay, that's two experienced maintainers recommending a renderer over page variants. Thanks for weighing in!
We don't need the query string as HTMX adds the header HX-Request: true
to all requests. I'll start to work on that approach as an alternative. I think we do still want title though so that the HTMX feature of swapping the title is available if appropriate but we turn it off by default.
Rendering the system messages in the response would mean that we wouldn't need a command to define the messages. The core systems of defining and displaying messages is available.
We would need to implement in a later issue either
- Always select and display system messages as additional inserted content
- Provide an operation for selecting the messages from the response and displaying them to be used with appropriate
All tests green, including new unit test for this subscriber
Nice simple solution @nod_. All tests green including the the added Ajax interaction test. What else do we need to move to RTBC?
All tests passed
PHP 8.4 test failed
Another advantage of the page variants is that SimplePageVariant
contains system messages as well as main content. That would allow the calling element to use hx-select-oob, or for us to add an hx-swap-oob to the response, and display error messages as well as the selected content from the response.
Maybe. That doesn't necessarily look simpler to me but I don't know the internals as well as @larowlan. The work in
📌
Process attachments (CSS/JS) for HTMX responses and add drupal asset libraries
Active
is based on returning an HtmlResponse
object, so would we be creating a simpler version of \Drupal\Core\Render\MainContent\HtmlRenderer
? I would think we would still need an event subscriber to divert the request to the alternate renderer.
Opened an MR that uses a default simple page with a header control for using a block page variant.
fathershawn → changed the visibility of the branch linting-in-progress to hidden.
fathershawn → changed the visibility of the branch 11.x to hidden.
fathershawn → created an issue.
@nod_ requested two additional test cases in the prior PR.
- htmx powered elements inserted by our legacy ajax
- verify that our attach and detach behavior methods fire
I've added a route to the test_htmx
module (/htmx-test-attachments/ajax) that creates markup for this test. In manual testing, htmx is inserted but does not process the inserted content. I think we need some glue code that calls htmx.process on markup inserted by our Ajax API.
The only way I see to verify that methods are called in the Nightwatch test is to use the callTracker assertion but this is deprecated, so I'm wondering if someone has a better approach?
I also added some documentation links to the JS files.
fathershawn → changed the visibility of the branch without-htmx-attachement-processor to hidden.
fathershawn → changed the visibility of the branch 3521173-process-attachments-cssjs to hidden.
fathershawn → changed the visibility of the branch asset-tests to hidden.
Thinking about the feedback on an additional processor service I realized that we should be able to simplify that whole part of the solution by adding some appropriate conditions to the existing HtmlResponseAttachmentsProcessor
. I'm hitting a wrinkle though which is that HtmlResponseAttachmentsProcessor
gets called more than once preparing the response to my test page. This is an issue because if I call setAlreadyLoadedLibraries
on the assets, then they stop getting added in subsequent runs.
Someone with more permissions must be needed to edit the JS Dependencies → page. I don't know if someone edited source or there are other text filters available, but I can't add additional headers and rows to the existing table.
Here's the needed data:
Repository https://github.com/bigskysoftware/htmx
Release cycle Releases are expected quarterly.
Security policies https://github.com/bigskysoftware/htmx/security
Security issue reporting https://github.com/bigskysoftware/htmx/security/advisories/new
Contact(s)
1cg →
,
fathershawn →
I hav two questions about merging drupalSettings.
ajax.js uses jQuery to merge the incoming drupalSettings value using the deep variation of .extend()
if (response.merge) {
$.extend(true, drupalSettings, response.settings);
} else {
ajax.settings = response.settings;
}
In my contrib implementation I didn't want to start with a reliance on jQuery so adapted code from https://youmightnotneedjquery.com. I've kept this pattern in the MR at the moment, but I see that jQuery.extend() is used nearly a dozen other scripts. What's the best way forward? I see
#3179551: Provide no-library equivalents of common/useful jQuery functions →
but it doesn't include .extend()
.
2. The ajax API provides two options for drupalSettings above. I currently simply merge. What is the use case for not merging incoming settings?
I've added a Nightwatch test, and all Nightwatch tests are passing.
I would recommend that not this child issue, but 📌 Process attachments (CSS/JS) for HTMX responses and add drupal asset libraries Active carry the change record. My reason is that although this adds the library, without the ability to bring in dependent CSS/JS it's not really usable yet. HTMX natively has properties to merge header tags but that clearly misses a lot of our JS.
If someone with more familiarity about the expectations around change records thinks that 2nd issue would be better place, I'll move the tag.
I have this working with in integration test. If anyone wants to see that locally, there is an asset-tests
branch in the fork, which is this issue, plus the htmx dependency issue merged into the core 11.x branch. That allows the test to run. I'll bring that test over to this branch once the htmx dependency work is merged.
That exception is thrown by \Drupal\oauth2_client\Service\Oauth2ClientService::getClient
which is called in each of those methods.
Tests passed 🎉
We have test failures that seem unrelated to this work: https://git.drupalcode.org/issue/drupal-3520723/-/pipelines/481254
fathershawn → created an issue.
Resetting status and moving to "Fixed" at @jurgenhaas suggestion so someone can comment and assign issue credits. Thought we needed to commit something to do that :)
I feel like this issue did it's job. We are now taking the learning from this work into new child issues on 🌱 Gradually replace Drupal's AJAX system with HTMX Active
MR is updated. Leaving as "active" until we have the security policy resolved
fathershawn → created an issue.
The storage is implemented by your own plugin class as the kind of storage varies by use case. My suggestion in #78 was to implement the storage methods but within them use a dpm to see the values rather than store them so you could see what was returned.
Alternatively you can use the provided storage trait and put a dpm() or ksm() call in the trait.
The snippet of code provided in #75 is from the StateTokenStorage
which itself is provided to reduce the code that developers need to implement in their plugins.
If the StateTokenStorage
file is empty of code then something is wrong with your code base. If you mean that nothing has been stored in the State system see my next paragraph.
The interface requires that you implement public function storeAccessToken(AccessTokenInterface $accessToken)
but while you are developing you don't have to store anything. You could simply print info from the token object to a message in your implementation of this method and not use the trait.
Linting complete.
Most tests fixed.
Only 1 failing test: ActiveLinkResponseFilterTest
. Suggestions welcome from anyone else who has worked in this area. I'm going to fire up xdebug to step through and puzzle out what is needed
So apparently it always has worked but no message of getting the token was ever placed anywhere to inform of success as you stated is supposed to occur
I have not been able to pull and print it from token storage however
The message display is a conditional in the StateTokenStorage
trait. I have just re-verified that it is functioning correctly.
/**
* Stores access tokens obtained by the client.
*
* @param \League\OAuth2\Client\Token\AccessTokenInterface $accessToken
* The token to store.
*/
public function storeAccessToken(AccessTokenInterface $accessToken): void {
$this->state->set('oauth2_client_access_token-' . $this->getId(), $accessToken);
if ($this->displaySuccessMessage()) {
$this->messenger->addStatus(
$this->t('OAuth token stored.')
);
}
}
I don't find any cloud based services for testing the client credentials flow, but you can configure and authorization code plugin to intereact with https://docs.wiremock.io/security/oauth2-mock#using-with-your-app
It was the last client that sends and gets a response
I have different results. In a clean install of Drupal, using latest version of Devel, the example plugin is both posted and uploaded in #66 along with inserting debugging functions in both sides of the code branch within getParsedResponse
also posted in #66, I get an expected denial response from USPS, also as posted when using random strings as client id and client secret.
I can try to add some more debugging output built into the module, but you could one of Devel's other functions there such dpm()
instead.
Rebased on this week's changes. Looking at test failures next
What I posted in #66 was the result of enabling the attached module in a new install of Drupal and altering the AbstractProvider code as shown.
I used the latest version of Devel which has this backwards compatibility code: https://gitlab.com/drupalspoons/devel/-/blob/5.x/devel.module?ref_type=h... but any of Devel's methods of outputing data would probably work. I use other methods to inspect values at runtime but this was my best idea.
I entered dummy client ID and secret and did a save and test. USPS responded with a message about improper credentials. What I posted works correctly, it just needs a real id and secret. If it is not working in your situation there must be some interaction with other code or the environment. I can't think of anything else.
If you have a ksm() call in both sides of the try/catch as shown in AbstractProvider then something is very much off with how the code is running in your setup. That's a dual path code branch and so one of them runs.
I can't tell more from here. Is there a Drupal community where you are? Someone you could pair with to debug this with you?
Making multiple large changes to your code is not the best way to debug. Single changes and test will get you where you want to be.
You didn't get a message in #62 likely because USPS returned an error. So let's move to this:
public function getParsedResponse(RequestInterface $request)
{
try {
$response = $this->getResponse($request);
ksm($response->getBody()->getContents());
} catch (BadResponseException $e) {
$response = $e->getResponse();
ksm($response->getBody()->getContents());
}
$parsed = $this->parseResponse($response);
$this->checkResponse($response, $parsed);
return $parsed;
}
With this in place, and this plugin (Also uploaded in a zip file as example 3)
<?php
declare(strict_types=1);
namespace Drupal\usps_api_example\Plugin\Oauth2Client;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\oauth2_client\Attribute\Oauth2Client;
use Drupal\oauth2_client\Plugin\Oauth2Client\Oauth2ClientPluginBase;
use Drupal\oauth2_client\Plugin\Oauth2Client\StateTokenStorage;
use League\OAuth2\Client\Provider\AbstractProvider;
use League\OAuth2\Client\Provider\GenericProvider;
/**
* Client credentials for USPS test endpoint.
*/
#[Oauth2Client(
id: 'usps_test_api',
name: new TranslatableMarkup('USPS Test API'),
grant_type: 'client_credentials',
authorization_uri: 'https://apis-tem.usps.com/oauth2/v3/token',
token_uri: 'https://apis-tem.usps.com/oauth2/v3/token',
)]
class UspsTestApi extends Oauth2ClientPluginBase {
use StateTokenStorage;
/**
* Creates a new provider object.
*
* @return \League\OAuth2\Client\Provider\GenericProvider
* The provider of the OAuth2 Server.
*/
public function getProvider(): AbstractProvider {
return new GenericProvider(
[
'clientId' => $this->getClientId(),
'clientSecret' => $this->getClientSecret(),
'urlAuthorize' => $this->getAuthorizationUri(),
'urlAccessToken' => $this->getTokenUri(),
'urlResourceOwnerDetails' => $this->getResourceUri(),
],
$this->getCollaborators()
);
}
}
I get this message after clicking "Save and request token"
You have two return statements in your getProvider code in #63
Maybe try inserting calls to ksm earlier in the stack after you fix that. You should be able to use that to find where it is failing