Ithaca, NY, USA
Account created on 13 February 2008, almost 17 years ago
  • Principal Software Engineer in Drupal Acceleration Team at Acquia 
#

Merge Requests

More

Recent comments

🇺🇸United States tedbow Ithaca, NY, USA

I am going to mark this as RTBC since @larowlan approved the merge request

🇺🇸United States tedbow Ithaca, NY, USA

UPDATE:
After I wrote most of what's below I thought, maybe I am overthinking it. Hopefully when we use Workspaces, our "publishing" will just be moving entity revision to another Workspace, and we can rely on Workspaces for handling transactions. If that is the case maybe we could just surround the current code in a transaction.

but I wonder if one quick improvement to make a deadlock in the tempstore less like would be to introduce \Drupal\experience_builder\AutoSave\AutoSaveManager::deleteAll() which would call a new \Drupal\experience_builder\AutoSave\AutoSaveTempStore::deleteAll and then that would call the existing \Drupal\Core\KeyValueStore\KeyValueStoreInterface::deleteAll()

I don't know that much about tempstore deadlocks but it seems like this \Drupal\Core\KeyValueStore\DatabaseStorage::deleteAll is 1 db operation so hopefully that would decrease our chances of deadlocks

Original, maybe overthinking🤔

I think this is good idea but also I wondering if there are thing we could do to make a deadlock more unlikely

However, there more may likely be deadlocks on the temp store when deleting the autosave.

Could we avoid this by making sure our tempstore is not read during the publish process?

For instance since currently you can't select which items you will publish, you have to "Publish all", could we stop the XB UI from working once the "Publish all" process is started. Nobody should really be making changes once you click "Publish all". Before we start saving the entities we have already loaded the auto-save states, $this->autoSaveManager->getAllAutoSaveList() , into memory so there is not chance new changes will affect the entity saves but another user using the XB UI would have no way of knowing that.

Maybe something like this

try {
$this->connection->startTransaction();
$this->autoSaveManager->lock();
foreach ($entities as $entity) {
    $entity->save();
}
$this->autoSaveManager->deleteAll();
}
catch(Exception $e){
   if (isset($transaction)) {
        $transaction->rollBack();
      }
      Error::logException($this->logger, $e);
      throw $e;
}

Then maybe the controllers that interact with the AutoSaveManager could do something like this at the start

if ($this->autoSaveManager->isLocked()) {
  throw new Exception('Experience is currently publishing.....');
}

We could put this a trait method, ensureNotPublishing().

Otherwise I don't think there is anything currently stopping somebody from continue to make XB changes while another user has just clicked "Publish all"

🇺🇸United States tedbow Ithaca, NY, USA

@wim leers thanks for finishing this up!

🇺🇸United States tedbow Ithaca, NY, USA

I think this is ready for review now

🇺🇸United States tedbow Ithaca, NY, USA

I got sidetracked by #33 but did push this forward and address some of the points.

@wim leers if you want to wait till give this another self review that is fine too

🇺🇸United States tedbow Ithaca, NY, USA

there are also big problems with our test coverage for non-empty list responses in `XbConfigEntityHttpApiTest`

  1. \Drupal\Tests\experience_builder\Functional\XbConfigEntityHttpApiTest::testAssetLibrary doesn't have test coverage for a non-empty response from \Drupal\experience_builder\Controller\ApiConfigControllers::list

    Trying to add test coverage for this in Auto-save code components Active I found it is not possible because the response has an openapi validation error.

    We have

              description: All available asset libraries
              content:
                application/json:
                  schema:
                    type: array
    

    But the "array" schema I don't think works because Looking at \League\OpenAPIValidation\Schema\Keywords\Type::validate it has

    case CebeType::ARRAY:
                        if (! is_array($data) || ArrayHelper::isAssoc($data)) {
                            break;
                        }
    
                        $matchedType = $type;
                        break;
    

    So it doesn't match "array" for associative arrays.

  2. \Drupal\Tests\experience_builder\Functional\XbConfigEntityHttpApiTest::testJavaScriptComponent does seem to test a non-empty list response but the openapi spec for /xb/api/config/js_component doesn't have GET specified so there is no openapi validation for the response
  3. \Drupal\Tests\experience_builder\Functional\XbConfigEntityHttpApiTest::testPattern does seem to have test coverage for non-empty list responses but Looking at paths./xb/api/config/pattern.get in our openapi there is
    patternProperties:
                      '^\\[a-zA-Z0-9_-]$':
                        $ref: '#/components/schemas/PatternPreview'

    So because of 🐛 Openapi.yml uses patternProperties which is not supported by our dependencies Needs work this doesn't actually trigger validation
    I have confirmed this by switching the above to $ref: '#/components/schemas/LayoutSlot'
    and testPattern() still passes 😭

So I think they are all broken in a different way 😱

🇺🇸United States tedbow Ithaca, NY, USA

Would be a good idea to refactor this test? looking at testPattern()don't all the entity types need to test the same things

  1. Authorization, already refactored into assertAuthenticationAndAuthorization
  2. Failed POST for Openapi validation
  3. Failed POST for requests that pass Openapi validation
  4. empty individual GET and empty list GET before entity creation
  5. Successful POST
  6. non-empty individual GET and non-empty list GET after entity creation
  7. 409 error for trying to POST another entity using the entity id key
  8. Failed PATCH for Openapi validation
  9. Failed PATCH for requests that pass Openapi validation
  10. Successful PATCH
  11. DELETE
  12. empty individual GET and empty list GET after entity deletion

Maybe this not the exact list but whatever the list is shouldn't be same for all entities?

Could we maybe make 1 test method like

 public function test(string $entity_type_id, array $openapi_invalid_data_cases, array $entity_invalid_data_cases, array $valid_initial_entity, array $expected_entity, array $valid_update_entity, $expected_updated_entity);

For

  • Failed POST for Openapi validation
  • Failed POST for requests that pass Openapi validation
  • Failed PATCH for Openapi validation
  • Failed PATCH for requests that pass Openapi validation

we could use array $openapi_invalid_data_cases, array $entity_invalid_data_cases for both cases

I think this would force use to have the same coverage for the all entity types.

If that is too many arguments to a test method we could also do,

 public function testOpenApiValidation(string $entity_type_id, array $invalid_entities, $valid_entity);
 public function testEntityiValidation(string $entity_type_id, array $invalid_entities, $valid_entity);
🇺🇸United States tedbow Ithaca, NY, USA

re #29

I found another problem with the existing test coverage 🐛 XbConfigEntityHttpApiTest Active

🇺🇸United States tedbow Ithaca, NY, USA

I think I started testAssetLibrary() and probably copied the existing testJavaScriptComponent() as starting point . So it makes sense that since testJavaScriptComponent() didn't test this testAssetLibrary() would also miss it

🇺🇸United States tedbow Ithaca, NY, USA

tedbow created an issue.

🇺🇸United States tedbow Ithaca, NY, USA

Working on the test changes I found this 🐛 XbConfigEntityHttpApiTest::testJavaScriptComponent doesn't change the component on the first PATCH request Active . would be good to get that little fix because we are updating that test function in this issue, so it is bit confusing. Because we would also want to ensure the PATCH doesn't affect the auto-saved version

🇺🇸United States tedbow Ithaca, NY, USA

Maybe I missing something but I think was just oversight

🇺🇸United States tedbow Ithaca, NY, USA

I think the clients existing call to /xb/api/autosave which uses ApiPendingChangesController could be used to if there are auto-saved changes. I am unclear if the client already has info about " current page has been published previously". but if it does there should not be need to be any back end changes.

This would need to be solved though 🐛 Once previewed in XB an entity with no changes will still show up in "Review x changes" Active

🇺🇸United States tedbow Ithaca, NY, USA

Looks good,

perform any rituals of passage he'd like 🤣

Preparing the rituals.....

but for now forgot how to perform the ritual that by-passes gitlab approvals, so will just try to get them 😜

🇺🇸United States tedbow Ithaca, NY, USA

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

🇺🇸United States tedbow Ithaca, NY, USA

@wim leers thanks for the review!

I am working on the need for test coverage changes https://git.drupalcode.org/project/experience_builder/-/merge_requests/5...

but pushed back/clarified on 2 other points. Assigning to you for the 2 points I disagreed but if want to wait on the test changes first thats fine

🇺🇸United States tedbow Ithaca, NY, USA

We might want to update `ApiPendingChangesControllerTest` to handle an Code Component in auto-save.

🇺🇸United States tedbow Ithaca, NY, USA

working on this

🇺🇸United States tedbow Ithaca, NY, USA

added openapi and 403 tests

🇺🇸United States tedbow Ithaca, NY, USA

Actually I still to test the openapi expected errors

🇺🇸United States tedbow Ithaca, NY, USA

I thought about handling cleaning up code components auto-save state if the config entity was deleted but I realized that is problem across entity types so I created 🐛 ApiPublishAllController does not handle since deleted entities Active

🇺🇸United States tedbow Ithaca, NY, USA

Thanks again for the help. Turned out it was other software installed on my laptop that was blocking certain requests

🇺🇸United States tedbow Ithaca, NY, USA

So I made another clone of Drupal core without Experience Builder and something similar is happening

If I run \Drupal\Tests\node\Functional\Rest\NodeJsonCookieTest::testPatch it fails in a similar way.

{"current_user":{"uid":"2","name":"w8ftf0xl"},"csrf_token":"mXeGJQEHIPybY_yeVvwuZSO3t4I6QGxNBgoBWDP9Vfw","logout_token":"o4SO3C55Y6Z7f9GThYpeGPvwtyajb1DcObPC7NBFl0A"}
Failed asserting that 200 is identical to 405.
/Users/ted.bowman/sites/xb-2/core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php:906

I simply changed a line in to show the body in EntityResourceTestBase. so the body on assert fail

$this->assertSame(405, $response->getStatusCode(), (string) $response->getBody());

so I think my findings in #10 were wrong about an asynchronous request. could it be that we making a patch request Drupal somehow redirects to the session route?

🇺🇸United States tedbow Ithaca, NY, USA

😱💩

Well I

  1. uninstalled Laravel valet and nginx
  2. switched to apache using https://getgrav.org/blog/macos-ventura-apache-mysql-vhost-apc
  3. rm -rf vendor + composer install
  4. restarted brew services restart httpd with every step
  5. run the tests from phpstorm and from the command line ../vendor/bin/phpunit ../modules/contrib/experience_builder/tests/src/Functional/XbConfigEntityHttpApiTest.php

still the test fails in the same way with "WTF, why is the response the token?"

🇺🇸United States tedbow Ithaca, NY, USA
  1. Thanks all. I am using Laravel Valet, which uses local php and nginx. I may switch back to https://getgrav.org/blog/macos-ventura-apache-mysql-vhost-apc
  2. The other thing I have noticed when step debugging.

    In this block of code, with xdebug on and put a breakpoint lines 3 and 4 and put a breakpoint in the first line of \Drupal\experience_builder\Controller\ApiConfigControllers::patch

     1. $token = $this->drupalGet('session/token');
      2.   $request_options['headers']['X-CSRF-Token'] = $token;
      3.  $response = $this->makeApiRequest($method, $url, $request_options);
      4.   $body = (string) $response->getBody();</li>

    when I stop at line 3 for patch request and then press "resume" I hit the breakpoint in ApiConfigControllers::patch but also the test advances to line 4 even if leave the server paused on ApiConfigControllers::patch. So it seems the request is asynchronous even though \GuzzleHttp\Client::request sets $options[RequestOptions::SYNCHRONOUS] = true;

Anyways I have wasted too much time on this. I am probably going to switch back to using local apache

🇺🇸United States tedbow Ithaca, NY, USA

On the test I am writing for Auto-save code components Active it also fails in the same way locally on a PATCH request. Not sure if this a coincidence. But in both these cases \Drupal\Tests\experience_builder\Functional\HttpApiTestBase::assertExpectedResponse is being called at least once before this with no problem.

🇺🇸United States tedbow Ithaca, NY, USA

I have started on this

🇺🇸United States tedbow Ithaca, NY, USA

Trying figure out sizing for this issue as far as what is needed for the back-end:

I see that Editing code components Active was just committed so there might be some concretes ideas of what the front-end expects from the backend now.

It seems like from discussion on HTTP API for code component config entities Active that we won't be implementing auto-save in in manner like content entities where the front will use something `\Drupal\experience_builder\Controller\ApiPreviewController` that returns a render markup and as a side effects does an auto-save, correct?

Instead we will be using a more direct method where the client will be sending up exactly the original and compiled js and css that it wants in the auto-save, correct?

I think if that is that case the auto-saving might be able to be a simpler controller wrapped around \Drupal\experience_builder\AutoSave\AutoSaveManager

Basically we would need a PATCH and GET(for reloading the editor) for the individual entity as saved in the AutoSaveManager

does that sound correct?

🇺🇸United States tedbow Ithaca, NY, USA

I think to find the number of pages or content entities in general that use a specific component we might need 📌 Ensure querying JSON nested values when parent keys are unknown is possible in all supported databases Needs work or more likely 📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active

Also one thing to note, 📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active won't directly get us the number of instances as it would just say if it was used at least 1x on a content entity.

from the summary

Display information about pages that currently use the code component.

Is it good enough to show how many pages use the component but not how many instances of the component there actually are?

So if a component is used on 2 pages but on 1 page it is placed 3x should the number shown be 1 or 4(1 page 1x + 1 page 3x)?

Also we want show indirected uses, correct? Meaning if Component A is used on 1 page by placing the Component A directly on the page and another page by placing Component B that imports Component A, that would show 2 uses, correct?

🇺🇸United States tedbow Ithaca, NY, USA

With my latest commit PropSourceEndpointTest should pass. Other test for the new stuff in this MR will probably break but a least we know $library_parser->setActiveTheme($active_admin_theme); is likely messing with the caching

🇺🇸United States tedbow Ithaca, NY, USA

🎉Ok figured out which line makes PropSourceEndpointTest, experience_builder_library_info_build()

pushed up a commit with this. But will paste it here:

// 💡 Returning here will make PropSourceEndpointTest pass. If \Drupal\experience_builder\Asset\XBFriendlyLibraryDiscoveryParser::setActiveTheme
  // is called in the next line,and we still return [], the test will fail.
  // I guess this results in a call to \Drupal\Core\Theme\ThemeInitialization::loadActiveTheme().
  // Could it be that calling loadActiveTheme during the hook makes the uncachable?
  //return [];
  $library_parser->setActiveTheme($active_admin_theme);
  // If we comment out the return above and make it here PropSourceEndpointTest will fail.
  return [];

I am not familiar with \Drupal\Core\Theme\ThemeInitialization::loadActiveTheme(). but assume it usually done 1x a request. Maybe calling in the hook again with a different theme makes the pages uncachable?

🇺🇸United States tedbow Ithaca, NY, USA

I chatted with @bnjmnm about this. PropSourceEndpointTest failed for locally that same way it does on gitlab. It passes for me locally on 0.x.

I tried various things and the only thing that worked was reverting that changes to experience_builder.redux_integrated_field_widgets.inc

I restored and put some debug asserts locally to see if I could figure out what was happening

This is the assert after the 2nd request for xb/api/config/component

// Ensure we get the exact same response contents as the first time.
    $secondResponseDecoded = $debugGetDecodeResponse();
    $this->assertSame(array_keys($firstResponseDecoded), array_keys($secondResponseDecoded));
    $this->assertSame($firstResponseDecoded, $secondResponseDecoded);

The last assert fails. Since the first passes we know the same the components are returned in the list
It is just that differ

It turns out components for blocks like block.user_login_block have different default_markup
so

- <input data-drupal-selector="form-qre8a3p5fsuun59wo6jradkbpeceewp-2ez9swbej9m" type="hidden" name="form_build_id" value="form-qrE8A3p5FSuUn59WO6jrADkbPEcEEWp_2EZ9sWbEj9M" />\n
 +<input data-drupal-selector="form-69vyw7nxd-eczqvemkgxhhdjy43obmtmxbgbicdmtdc" type="hidden" name="form_build_id" value="form-69vYw7nxd-ECzqveMkGXHHdJY43obMTMXbGbiCdMtdc" />\n

so the identifier for the form. I guess since cache was not hit the form was regenerated. I guess maybe that is not very helpful 🤷

🇺🇸United States tedbow Ithaca, NY, USA

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

🇺🇸United States tedbow Ithaca, NY, USA

Starting looking at the issue. I am not clear why the schema in the summary using a mapping for CS and JS with sub elements for source and compiled

js:
      type: mapping
      label: 'JS'
      # The asset library may contain only CSS, no JS.
      nullable: true
      mapping:
        original:
          type: text
          label: 'Original JS as entered by the human author'
        compiled:
          type: text
          label: 'Compiled JS code, compiled from the original'

where as in the just finished Config entity for storing code components Active we use

    source_code_js:
      type: string
      label: 'JavaScript Source code'
    source_code_css:
      type: string
      label: 'CSS Source code'
    compiled_js:
      type: string
      label: 'Compiled JavaScript'
    compiled_css:
      type: string
      label: 'Compiled CSS'

My guess is there is not particular reason for the difference. The mapping might make more sense but I think consistency is important, so I am going to go with what we used for experience_builder.js_component.* unless someone lets me know why these should behave differently

🇺🇸United States tedbow Ithaca, NY, USA

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

🇺🇸United States tedbow Ithaca, NY, USA

@catch my general concern from #11 that you were responding to in #15, is not an issue for me if we implement #12. Basically just only do this for demo_mode, which I think practically this means only do this for Drupal CMS.

I agree with this for someone that installs the alpha module themselves.

So if this only affects demo_mode/Drupal CMS then I think others can use the alpha module like any other alpha module

...Trying to create an ephemeral demo on what are supposed to be production installs is just inviting catastrophic data loss or inability to update, in the various myriad forms those issues could appear.

I haven't been involved in the discussion around but I can see your point.

Is there another issue where I can see the discussion?. If demo_mode is done I want to see if there is anything we can do to lesson the risk in XB itself

🇺🇸United States tedbow Ithaca, NY, USA

working on this

🇺🇸United States tedbow Ithaca, NY, USA

Need to check demo_mode and update demo mode description

🇺🇸United States tedbow Ithaca, NY, USA

I chatted with @phenaproxima about this. It seems like if the script is updated to only uninstall if experience_builder.demo_mode is true, then wiping with each update is ok. Add a code-only (not changeable via UI) "demo mode" flag that Drupal CMS can set and that future XB MRs can use to constrain XB functionality Active explicitly added this for Drupal CMS so I can't imagine others will use it. It seems like it is up to Drupal CMS to provide the message around.

Just to be careful I think we should update the description of demo_mode in config/schema/experience_builder.schema.yml to explain this behavior. This will avoid people finding this setting and not understanding what the implications of setting is, and this issue is definitely adding more implications for setting this flag.

🇺🇸United States tedbow Ithaca, NY, USA
  1. I agree with #10

    I think MR works well for Drupal CMS but it seems like getting testers for XB to be a pain.

    This seems like a general problem that if you are using a Alpha version of the module each update is going to risk problems, maybe very bad ones, when you update the module. But that seems to risk you would also take with using any alpha module.

    Say we have testers that are using Alpha1 they have made some complex pages, global templates changes, global css, and code component changes.

    They find a small UX bug and we fix it in Alpha2. They want to test Alpha2 and give feedback. By default they would have to rebuild everything that they had already done. what if we they rebuild everything and bug is not there. Is it not there because fixed in Alpha2 or is it not there because when they rebuilt everything they did it just slightly different and they didn't hit the bug and won't have hit in Alpha2 either.

    Upgrading between Alphas doesn't guarantee that we will break the data model and your site won't work. It just means it might. If we did this I can't imagine anyone would try to do anything complex with the alpha versions of XB, especially after we wiped their data the first time(maybe they didn't pay close attention during composer update). I think we do want risk takers to try complex with XB. That we would more confident to move onto alphas

  2. what happens if someone updates XB with Automatic Updates? They wouldn't see an option to not uninstall even if we added, right? I could see there being a large intersection of people who don't want to deal with Composer, so they use AU, and those who don't want to deal with twig templates, so they use XB.
🇺🇸United States tedbow Ithaca, NY, USA

Look at test errrors

🇺🇸United States tedbow Ithaca, NY, USA

Long-term, I think this ought to be implemented as \Drupal\Core\Config\ConfigFactoryOverrideInterface.

Are we sure long term we will need this at all if rely on wse_config?

We have requirement out auto-save js components but this seem like more a UX requirement than a specification for how this would handled as far as backend storage. Could we just do real config saves, to back up work without user action and just let wse_config be responsible for making sure that doesn't pollute the live site? (relating 🌱 Identify roadblocks to staging config in Workspaces (e.g., via wse_config) Active to this issue for that reason)

I think in 🌱 Research: Possible backend implementations of auto-save, drafts, and publishing Active the 2 main reasons we decided we needed some auto-save mechanism is that not a real entity save was

  1. Bypassing Validation: saving work in progress that is not valid
  2. Avoid the saving overhead so we could save frequently

For 1 we could still use a simple auto-save if the validation does not pass but a real entity save if it does. In the case of saving content entities we are not using the auto-save state, which might not be valid, to then render the content entity outside of the XB UI itself or to render that entity in the XB UI for another entity besides itself. Would that be the case for the JS components? What would even happen if we tried to a render JS component config entity overridden by ConfigFactoryOverrideInterface in XB UI for another entity, say Page content entity, if the auto-save state is not valid?

For 2 do we have the same overhead concerns for JS Component config entities as we do for Content entities. If XB is going to be the default way some sites add new and edit existing Content and as well as JS Component, it still seems likely that sites will not be adding as many JS components as they would content entities.

🇺🇸United States tedbow Ithaca, NY, USA

I think to find the number of pages or content entities in general that use a specific component we might need 📌 Ensure querying JSON nested values when parent keys are unknown is possible in all supported databases Needs work or more likely 📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active

I will research this further

🇺🇸United States tedbow Ithaca, NY, USA

I am wondering if it would make sense to start an MR hear that would add a ComponentUsageCalculator service. Because we are going to need some way to find how many usage particular component has. I guess this will be across content entities using a Json field query, but also we would need to loop take into account instances inside `PageTemplate` .

For PageTemplate entities is it just important show a list of which regions would be affected? Would this only for the active theme? I can imagine in some sites they make start with 1 theme, places some code components in regions but then decide to switch to another theme and not bother to uninstall the other theme. But in other sites they may have custom code or a module like https://www.drupal.org/project/theme_switcher switches themes.

🇺🇸United States tedbow Ithaca, NY, USA

Clicking view swaps the modal to a deeper view of which
pages are affected

What about if

  1. You have 2 existing code components, A & B
  2. the code component B is used inside code component A
  3. Code Component B has been edited but Code component A has not

Would the "deeper view " show that Code Component A has been affected? Would it should all pages where code component A is used but not B directly? Because these pages would have changes resulting from B changes
Would the number 46 in the example image take into account all Component B placements?

🇺🇸United States tedbow Ithaca, NY, USA

This number shows the total number of instances that will be affected by the basic card component update.

So is this if you are making a new component or editing an existing it shows how many times the JS component is place

So for example:

  1. New or existing Component, but no instances in Unpublished Changes or already published = 0
  2. Existing Component
    Unpublished changes: 2 instances added in Page A, 1 instance added in Page B
    Already Published: 3 instances in Page C,
    = 6
  3. Existing Component:
    Unpublished changes, 1 instance added in Page A, 1 instance removed in Page C
    Already published 3 instances in Page C,
  4. = 3? because there will be instances once "Publish all" is submitted?

🇺🇸United States tedbow Ithaca, NY, USA

Just a start but I will unassign myself as others will be starting there day tomorrow before me.

🇺🇸United States tedbow Ithaca, NY, USA

I bumped it to major only because I think we should decide sooner rather than later if this going to need changes to the system outside of Code Components. I made possible way we could do this that might just apply to the Code Components code

🇺🇸United States tedbow Ithaca, NY, USA

Will work on this

🇺🇸United States tedbow Ithaca, NY, USA

I was just on call with @balintbrews and @lauri regarding auto-saving Code Component. I just wanted to get my understanding down in regard to the usage of auto-saved components, because this would intersect with out parts of XB.

This is my understanding at least for 🌱 Milestone 0.2.0: Experience Builder-rendered nodes Active

  1. If a code component is only in an auto-save state and has never been published, it will not be available in the component list in the library of the XB ui. Meaning you can't place an code component on the page/node if it is only in auto-save
  2. if a code component has been published, it will be available in the component list in the library of the XB ui, it can be placed
  3. if a code component has been published but the editor is currently making changes that are in auto-save, the version available in the component list in the library of the XB ui that can be placed and the version for instances that are already placed, is the published version and is not affected by the auto-save state. This auto-saved version of Code Component also would not affect any instances on page/nodes/global regions that are already been "Published"(in the XB process of "publish all")
  4. for an already published version of a Code Component, if the editor has an auto-saved version, once the auto-saved Code Component is published all versions in both pending XB page/nodes/global regions, and already published XB page/nodes/global regions would use the updated version. There would no option to use the previous version
🇺🇸United States tedbow Ithaca, NY, USA

@Wim Leers re #54 ok. yeah I reread the issue in 📌 [PP-2] Don't store page template model data in auto-save for an entity Postponed . I missed this part

The values are filtered out in \Drupal\experience_builder\Controller\ClientServerConversionTrait::clientModelToServerProps but we are storing more in the autosave/tempstore than we need to.

I thought there would be conflict in "Publish All" phase because each entity could have its own copy of the global regions that could conflict. Now I see it is just extra data we don't actually use. So +1 on moving it to "Polish" section

🇺🇸United States tedbow Ithaca, NY, USA

Finished going through issue queue for 6. Save (draft) content section

I think we are close but we mind unknown issues once [PP-1] Implement the "Publish All" button Postponed is completed as they will allow use to do the whole process. For this to include entity fields we will need the 2.1. Content editing of meta fields section to be done too

As far as know is 🐛 Some components cannot be used in XB because UI prevents SDC props being named `name` Active isn't required for 0.3.0

🇺🇸United States tedbow Ithaca, NY, USA

FYI 📌 CONTRIBUTING.MD is nearly impossible to follow without insider drupal knowledge Active . I am not following either of these but so not sure if they are dealing with the same sections of the file

🇺🇸United States tedbow Ithaca, NY, USA

I merged this because it was random fail in another e2e test. Since this MR adds 1 new e2e test, not failing, I think it is fine to go in.

Will make an issue listing known random e2e tests

🇺🇸United States tedbow Ithaca, NY, USA

Renaming the title case in body sees this in the queue, to give a better idea of what it does

🇺🇸United States tedbow Ithaca, NY, USA

component-operations.cy.js failed but since we just added a new test I can't see how this is related

retesting

🇺🇸United States tedbow Ithaca, NY, USA

Added issue for the "Save (draft) content" section. I am still looking for more issues to add

🇺🇸United States tedbow Ithaca, NY, USA

Assigning to myself for reminder but if anyone else wants to make follow-up MR please assign yourself. You can assign back to me for review

🇺🇸United States tedbow Ithaca, NY, USA

I assigned @traviscarden to review because we needed openapi.yml changes but now those have been fixed in 🐛 open #/components/schemas/LayoutPreview should require properties Active

So @jessebaker and myself already approved this

🇺🇸United States tedbow Ithaca, NY, USA

I am not not really sure why this pipeline which just has new expection thrown https://git.drupalcode.org/project/experience_builder/-/jobs/4018805

Does report the error response

where as this one https://git.drupalcode.org/project/experience_builder/-/jobs/4018805
which was from the other issue and the reason I made this issue does not

Anyways bumping down to minor because this just might be me not understanding our e2e jobs will log errors for requests(not tests) that fail

Production build 0.71.5 2024