🇮🇹Italy @gambry

Done.

Hi @dimitriskr . TBH I completely forgot about this project :D
If it still brings benefit to the community, why not. I'll add you as a co-maintainer.

Moving this back to Needs Work for passing the $existing intent together with $content.

@nicxvan I don't know. What's the policy in this scenario?

TL;DR: I'm with current changes without alteration.

I spent quite a while, this is my understanding:

The original aim of the changes proposed is to allow subscribers to react by altering application states, while not changing the content. This seems a reasonable no-disruptive approach, since any default content provided will be processed as expected by their providers.

Passing $existing
Passing "Existing" is harmless per-se, but it will require splitting the loop in two parts and dispatching this event after the first loop loading the entities. Or similar approaches.
The benefit of having $existing is to let event subscribers react when something exists.

IMO the benefits don't pay back for the work that needs to be done. Subscribers can anyway load the entities by themselves and checking if they exist. Loading entities will be cached so when re-run by the Importer class it will be seamless.

Allowing subscribers to remove items
Subscriber will be able to remove items from the list (let's not give the chance to alter content too 🙏). It won't require many changes and it could be done in a data-integrity safe way.

However if I'm a recipe owner I want to ultimately be responsible for my content. I expect for it to be imported and if there is a failure I'm ultimatelly responsible for fixing it.
If I'm a contrib module owner, and my module weirdly handle content I should be ultimatelly responsible to fix my 💩 (like Trash module is trying to do).

Allowing any subscribers to manipulate the list without notifying the rest of the subscribers - which would require a new event :D - seems dangerous to me.

Conclusion
For this reason I believe the current implementation is the most obvious, covering the majority of usecases and reducing risks of overcomplicating the import flow.

Hi, Gab from Drupalcon Signapore Contribution Day. Looking at this.

@Zoocha Will here you go.

Drupal Wordmark (blue)

Drupal 10 (blue)

I'm not sure what format you need them, however higher resolutions can be found/obtained here: https://drive.google.com/drive/folders/13XdLr2kvN7GrvYC0tUtxSwLXqE4nfcA4

I do agree to keep Druplicon, and adding the new logo and/or the "DRUPAL (TM)" options.

@Zoocha Will I'm not a designer, but I'll try to come up with a Drupal 10 alternative.

gambry → created an issue.

fathima.asmat → credited gambry → .

smustgrave → credited gambry → .

Mention support for custom payload, card or event is now supported for Dialogflow. https://www.drupal.org/project/api_ai_webhook/issues/3333883#comment-149... ✨ Response with custom payload / richContent Fixed

Fixed! Thanks a lot!

Fixing the test.

While tests run, a bit of documentation.

There are two main ways custom payload or event can be added to the fulfilment:
$this->response->getFulfillment()->addMessage()
$this->response->setIntentRespose()

Let's kick tests.

@dotist I'll post it in here too, in case you haven't seen it.

Based on your PR, I created this one: https://github.com/gambry/dialogflow-webhook/pull/3
Please have a look if it has got everything you had in mind, so I can merge it and we finish working on this issue.

Thanks!

@dotist thanks for your work. The patch can be accepted as it is, most of the changes are not really needed (all comments, creation of unused variable, dockblock not needed).

+++ b/modules/chatbot_api_apiai/src/IntentResponseApiAiProxy.php
@@ -80,8 +80,17 @@ class IntentResponseApiAiProxy implements IntentResponseInterface {
+  public function setIntentResponse($response, string $type = 'messages') {

This is a risky change of API. If committed, integrations may break until developers upgrade their code.
What about something like setIntentResponse($data, string $type = 'text') {}, with text being the default. So we make this backcompatible?

We need to make sure we don't break things and new code is tested.
I'll comment changes to gambry/dialogflow-webhook in its own PR.

Thanks!

Adding the correct tag for Drupal Global Contribution Weekend 2023.

Adding the correct tag for Drupal Global Contribution Weekend 2023.

Adding the correct tag for Drupal Global Contribution Weekend 2023.

Adding the correct tag for Drupal Global Contribution Weekend 2023.

Adding the correct tag for Drupal Global Contribution Weekend 2023.

Adding the correct tag for Drupal Global Contribution Weekend 2023.

Adding the correct tag for Drupal Global Contribution Weekend 2023.

Adding the correct tag for Drupal Global Contribution Weekend 2023.

Adding the correct tag for Drupal Global Contribution Weekend 2023.

Adding the correct tag Drupal Global Contribution Weekend 2023.

Adding the correct tag got Drupal Global Contribution Weekend 2023.

Adding the correct tag got Drupal Global Contribution Weekend 2023.

Adding the correct tag got Drupal Global Contribution Weekend 2023.

Adding the correct tag got Drupal Global Contribution Weekend 2023.

Adding the correct tag got Drupal Global Contribution Weekend 2023.

(leaving the original one though. Plz remove it if not needed for internal use)

Swapping with the right tag: ContributionWeekend2023.

Re-testing #93. Also adding other DB types to validate this is not a DB type and we haven't tested this patch against other DB types for almost 1 year, so to create a newer baseline.

Trying to resurrect this at Drupal Global Contribution Weekend.

Applying and running the test locally, to see what can be done.

Swapping the Drupal Global Contribution Weekend 2023 tag with the correct one.

Swapping the Drupal Global Contribution Weekend 2023 tag with the correct one.

Swapping the Drupal Global Contribution Weekend 2023 tag with the correct one.

Adding Needs Issue Summary Updates due notes on #58, still pending confirmation if anyone can test my assumptions?
Also keeping Needs Tests since the tests I posted #46 don't focus on system.site page.front changes we are trying to achieve.

@DieterHolvoet yeah, you are right. Probably I was running on top of the 1503146-drupalisfrontpage-does-not branch.

So, on Basic Settings form:

• on 9.4.x: when setting /page-alias (or its /node/ID), system.site page.front is /node/ID
• on this issue branch: when setting /page-alias (or its /node/ID), system.site page.front is /page-alias

This patch address this issue, which mainly focus on decoupling the config from the content. Using an alias rather than an internal content path in config is preferable. 👍

RE url redirects:

• on 9.4.x: when setting /page-alias as frontpage, visiting /page-alias does not redirect to /
• on 9.4.x: when setting /page-alias as frontpage, visiting /node/ID does not redirect to /
• on 9.4.x: when setting /node-ID as frontpage (page WITHOUT an alias), visiting /node/ID does not redirect to /

So my understanding is yes, there is an issue with the form, but not with URL redirects? In this case we still need to update the IS and remove all mentions to "redirect" since are misleading.

I'm tempted to close this as "cannot reproduce". The issue doesn't seem to happen anymore, at least in 9.4.x.
The steps to reproduce don't trigger the issue.

The behavior either you use the system path or an alias is exactly the same, as exception of the scenario where you set a system path through the form which is replaced with its alias if one exists.

Feel free to re-open, showing exactly what the issue is and how to reproduce it.

Thanks everyone!

Putting in the form /node/1 converts it to its alias /ciao-mamma.

% ddev drush cget system.site
_core:
  default_config_hash: VDJxTZtQR21qB4lvOq8zszJZLvLKrSPQpdn2E3T71Ww
langcode: en
uuid: 6db72c80-119d-4beb-9b22-f0c310f1e3df
name: 'Drush Site-Install'
mail: admin@example.com
slogan: ''
page:
  403: ''
  404: ''
  front: /ciao-mamma
admin_compact_mode: false
weight_select_max: 100
default_langcode: en

If I understand correctly the issue you are describing, then no. The alias is not changed into the node path in the Basic site settings form.

@DieterHolvoet it doesn't on Standard profile?

With standard:

1. Create a Basic Page
2. add an alias
3. in Basit Site Settings, configure the alias as front page
4. visit /

I'm not sure I understand correctly this bit of the IS:

causing the front page path to not redirect to the root path if an aliased path is used. Internal paths like /node/1 work fine.

With Standard profile, if I visit a /node/1 path configured as front page there is no redirect. Neither with the system path /node/1, nor with an alias. Anyone has got different behaviors? If I'm right, do we need an IS update?

Oops. Needs Review, for the testbot.

Before checking those failures, here a test-only patch. setting it for Needs Review just to trigger the testbot. This is Needs Work.

In this commit:

• Include CS fixes from #42
• Addressing testing failure from #42
• Changing visibility to PathMatcher::checkFrontPage(). As this is not in the interface, it shouldn't really be public. So changing it to protected. If needs to be public, then we better add it to PathMatcherInterface too.

This still needs test.

Long time has passed and this issue now requires IS updates either to reflect changes required and properly scope out solutions based on 9.x OR focusing this issue to D7 and moving D9 work somewhere else ( #3170184: Homepage replaced by login page, why ? → maybe?).

Hey @scott_euser have you seen work on the related issue?
Can we merge the two issues, and close the duplicate?

Wonderful work, well done everyone!

Below my feedbacks.

1. +++ b/core/modules/file/src/Element/ManagedFile.php
   @@ -352,8 +353,17 @@ public static function processManagedFile(&$element, FormStateInterface $form_st
   +    // Add states to ajax wrapper so states.js can potentially attach this
   +    // element as a Dependent.
   +    $attributes = '';
   +    if (isset($element['#states']) && !empty($element['#states'])) {
   +      $attributes = new Attribute([
   +        'data-drupal-states' => json_encode($element['#states']),
   +      ]);
   +    }
   +
        // Prefix and suffix used for Ajax replacement.
   -    $element['#prefix'] = '<div id="' . $ajax_wrapper_id . '">';
   +    $element['#prefix'] = '<div id="' . $ajax_wrapper_id . '"' . $attributes . '>';
   

   $attributes is first a string, then an Attribute() instance. Why don't we make it an Attribute() instance since definition, and maybe we create it with [id=$ajax_wrapper_id] and so print all prefix element attributes in the same way?

2. +++ b/core/modules/file/tests/src/FunctionalJavascript/FileManagedStateTest.php
   @@ -0,0 +1,60 @@
   +  protected function setUp() {
   +    parent::setUp();
   +  }
   

   I don't think this is needed then, as it's overriding the parent::setUp() just to call it. :D

3. +++ b/core/modules/file/file.js
   @@ -36,6 +36,10 @@
   +      $(document).trigger('state:visible');
   

   This looks to handle only the visible, so I tried some other states and something looks broken. Apparently all states are applied to the parent wrapper and not to the input element itself. So some states work (visible, invisible, disabled) but other like required don't.
   States changes should affect the element and not the parent.

#15 I don't understand your solution.

to check if the frontpage url is a part of the request url

An URL beginning (or ending) with the frontpage path is totally unrelated to this issue.

The issue is about drupal returning FALSE on either D7 and D8 on checking if path is frontpage, when the site frontpage setting (system.site.page.front/site_frontpage) is an alias.

To reproduce in D8:

1. Pick up a node and add its alias to the Basic site settings Default front page i.e. /foo/bar
2. Getting system.site.page.front returns the internal path /node/123, while Basic site settings Default front page shows its alias
3. Go to the page and drupal correctly recognises the page as front page i.e. it has the body class "path-frontpage"
4. drush cset syste.site page.front /foo/bar, use same alias as step 1
5. re-do step 2 and drupal doesn't recognise the page as front page i.e. the body class path-frontpage is missing

Step to reproduce in D7 are similar, just use drush vset site_frontpage and paths don't start with "/".

Proposed solution:

• D8: use \Drupal::service('path.alias_manager')->getPathByAlias($path); on PathMatcher::isFrontPage() for matching with system.site.page.front
• D7: use drupal_get_normal_path(variable_get('site_frontpage', 'node'))); on drupal_is_front_page()

EDIT/UPDATE: I've removed the "Bug" type as I don't think is a bug, but it's not clear if system.site.page.front MUST be an internal path or it can be anything. The form itself is misleading (you add an alias, it stores its internal path but still show the alias on the form field after submission) and even if we make it clear it's easy for the developer to forget it must be internal, store an alias and break the website.
Feel free to restore "Bug".
Besides I'm thinking internal path can change - for instance between different environments - while alias can stay the same - you create a node with the same title/alias -, so I can see why a developer want to store the front page url using alias and not internal path. I'm more keen to restore the "bug" type... thoughts?

I don't think it is a bug. The problem is the expected behaviour must be updated as frontpage can be set in several ways directly to the variable site_frontpage (Features, drush, settings.php, etc.) and not passing from the form.

The solution above is suggesting to move the alias-to-internal translation step from the administrative form to the drupal_is_frontpage() function, and so from a mapping approach on saving the form to an universally checking one on loading the page.

Then whatever is the site_frontpage value - alias or internal - the drupal_is_front_page will always return correctly.

EDIT/UPDATE: I've carefully read all the comments and I can see someone is tripping over some unclear bugs. I can't reproduce them on 7.x, haven't tried on 8.x but linked code is clear. The only issue I see here is still the one on the description of the issue related to drupal_is_frontpage_code(). Other core modules check both internal and alias paths on evaluating a page (i.e. block module on block_block_list_alter() when matching request path against $block-pages).