Even with the following request
POST https://drupal11.ddev.site/conductor/proxy/v3/content-outline/content-generation?draft_id=3b3f52bf-88f0-43f2-a14e-a2ed75e83856
Content-Type: application/json
Accept: application/x-ndjson
It seems I'm unable to get a stream. I only get a request with the Content-Type: application/x-ndjson
This commit only includes a very draft and dirty code to convert the ndjson into a regular json:
$stream = "[" . str_replace("}\n{", "}\n,{", (string) $psr7Response->getBody()) . "]";
try {
$json = json_decode($stream);
}
catch (\JsonException $e) {
$this->logger->error($e->getMessage());
}
return new JsonResponse($json, $status, $headers);
This code isn't production ready code. I just want to test different requests.
My feeling is that the request is not correctly requested (from the client) or served (from the API). I'll keep digging.
Context:
NDJson add "Newline Delimiter JSON", so a NDJSON will look like this:
{"id": 1, "name": "A"}
{"id": 2, "name": "B"}
{"id": 3, "name": "C"}
It fails because it expects to be a valid json.
Core doesn't support ndjson, I've found 2 libraries for NDJSON, but one requires many dependencies and the other one requires to save the stream into a file and then parse it.
Or maybe we can commit this to unblock the issue and create a follow-up to configure all components properly.
Thanks for testing.
@kirsten-pol, do you mind to point which ones requires the ckeditor to add the pattern? I'm afraid have no idea which ones requires it 😅
Yesteday I've created this issue and MR: https://www.drupal.org/project/canvas/issues/3551787 📌 Re-introduce entityId and entityType into drupalSettings.canvas Active after a chat with Balint.
Tugboat is also broken because DDS is using a lot of canvas.module/textarea
I've created an Issue: https://www.drupal.org/project/demo_design_system/issues/3552037 🐛 textarea is not a valid json_schema anymore Active and a MR here: https://git.drupalcode.org/project/demo_design_system/-/merge_requests/114
This MR, hopefully, fixes the Tugboat issue.
I've created this issue because in the 📌 Update Conductor to Canvas RC Active issue we've found that these 2 variables no longer exists in RC1.
I've seen that there is only one route processed (besides the /canvas/api), and is /canvas. This route is canvas.boot.empty and this route doesn't have any parameters. All the /canvas routes are processed in the CanvasPathProcessor
Is this the expected behavior? If so, CanvasController::__invoke will always have null parameters for entity and entity_type?
isholgueras → made their first commit to this issue’s fork.
I think this is no longer an issue, or at least I'm unable to reproduce it with the steps.
The drupalSettings.conductor.accountId is set in a js_settings_alter, but originally was set in a js_settings_build.
I've changed it to the js_settings_alter because I had some issues with cache, but I think that's now fixed.
After seeing that there is a
new release for DDS →
that handle the Canvas rename, and after removing the canvas_dev_standard module as canvas does, it's ready for review and merge.
After applying the rename on 📌 Update XB occurrences to Canvas Postponed , Tugboat was merged without changing it to canvas.
isholgueras → made their first commit to this issue’s fork.
I've tested it in 2 different Drupal 11
- An existing Drupal 11 with Canvas and Conductor installed and configured. Replaced the existing module by the zip version, cleared caches and everything work.
- A basic Drupal 11 with Canvas. Reinstall the site, place conductor into web/modules/contrib/conductor and install it. Everything works as expected too.
RTBC for me.
If the response from conductor has the Content-Type header with more attributes, like application/json; charset=UTF-8, the json is not correctly parsed.
Thanks for the work @Rajeshreeputra! I'm going take your job and continue it.
isholgueras → created an issue.
I've reviewed the code, looks good. I've also reviewed the functionality and it works as expected.
For me is a RTBC
Ready for review.
isholgueras → created an issue.
isholgueras → created an issue.
I think it's ready for a first review.
The steps to test is:
- Enable the module
- Add to the config page the api key and secret
- Make requests to the conductor api resources like using the route /conductor/api/{conductor-resource}
- /conductor/api/accounts
It should accept GET, PUT, POST and DELETE requests.
Core is currently doing for /system/files/ route.
It uses an InboundPathProcessor to get the route of the file, set it to a query parameter and return just the /system/files to allow the route system to match the route, no matter how many / the file has, because now is in a query parameter.
class PathProcessorFiles implements InboundPathProcessorInterface {
/**
* {@inheritdoc}
*/
public function processInbound($path, Request $request) {
if (str_starts_with($path, '/system/files/') && !$request->query->has('file')) {
$file_path = preg_replace('|^\/system\/files\/|', '', $path);
$request->query->set('file', $file_path);
return '/system/files';
}
return $path;
}
}
Then, in the controller, it gets the target by accessing the file query parameter.
$target = $request->query->get('file');
isholgueras → created an issue.
It's ready for review.
isholgueras → created an issue.
I have one idea to write a test for this. If it works I think it will be easier to refactor this.
Is this still an issue? I've went through:
1. Install Standard install profile
2. Install experience_builder
3. Create article node — let's assume it's node 1
4. Install xb_dev_standard
5. Go to /node/1 and ...
... add a new image (without the media module)
6. Click "Review 1 change" and "Publish"
7. Visit /node/1 and see my image
I can investigate deeper on when this was fixed, but I think this can be closed.
I still have some time 🤜🤛
I've added some concerns in the MR about duplicated conditions and code that can be removed.
The only difference that this MR brings is the exception logged in the database and is step forward and we would have to delete this sooner or later. I'm OK with this.
I've fixed one phpunit test and all should be green now.
isholgueras → made their first commit to this issue’s fork.
Oh, I see it now. Ok, make sense then. I thought it was leftover code.
No, it doesn't make sense to rename it. I think it can be closed.
Thanks!
isholgueras → created an issue.
After talking to @wim, I've realized that #10 📌 Move `PropSourceEndpointTest` into new `XbConfigEntityHttpApiTest::testComponent()` Active is adding a new task, so it's not ready.
The current MR1379 is only moving the test
isholgueras → changed the visibility of the branch 0.x to hidden.
isholgueras → changed the visibility of the branch 3510896-move-propsourceendpointtest-into to hidden.
isholgueras → changed the visibility of the branch 3510896-move-propsourceendpointtest-into to hidden.