Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

draft messenger documentation #195

Closed
wants to merge 31 commits into from
Closed

draft messenger documentation #195

wants to merge 31 commits into from

Conversation

galvani
Copy link

@galvani galvani commented Aug 10, 2023

patrykgruszka and others added 26 commits March 24, 2023 10:32
Merge main into 5.x branch ready for release
email_transport queue is now called just email
Move Email config / DSN settings to Configuration section
@galvani galvani marked this pull request as ready for review August 10, 2023 13:34
@escopecz escopecz requested a review from RCheesley August 10, 2023 14:18
Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Jan! It loos good! I have a few questions. Also, please rebase this against the 5.x branch.

Comment on lines +55 to +61
.. code-block:: php

'routing' => [
\Symfony\Component\Mailer\Messenger\SendEmailMessage::class => MauticMessengerTransports::EMAIL,
\Mautic\MessengerBundle\Message\PageHitNotification::class => MauticMessengerTransports::HIT,
\Mautic\MessengerBundle\Message\EmailHitNotification::class => MauticMessengerTransports::HIT,
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this information for? Will users need to configure it themselves or change it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is to show how routing is done so it is easier to understand why we configure just the one transport a little lower. so they understand the routing
message => route => transport => handler

Comment on lines +43 to +48
.. code-block:: php

MauticMessengerTransports::HIT => [
'dsn' => 'sync://',
'serializer' => 'messenger.transport.jms_serializer', // Smaller payload
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where should users put this? Or isn't this the default behavior?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have described it better now.

docs/messenger/messenger.rst Outdated Show resolved Hide resolved
@escopecz escopecz requested a review from fedys August 10, 2023 14:32
@galvani
Copy link
Author

galvani commented Aug 10, 2023

@escopecz I have fixed the issues and updated.

Copy link
Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems there are unrelated comments in this PR now. But looking at them I think the new doc will have to be registered in docs/index.rst too. I'm also still new to this new doc. @RCheesley might tell for sure.

@RCheesley RCheesley changed the base branch from main to 5.x August 11, 2023 16:52
@RCheesley
Copy link
Member

This PR still needs rebasing to 5.x please? Thanks!

@RCheesley RCheesley changed the base branch from 5.x to main August 11, 2023 16:53
Copy link
Member

@RCheesley RCheesley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making a start on the docs @galvani !

Some general feedback:

  1. Please move messenger/messenger to configuration/messenger - that is where a user would go to find information about configuring the email / messaging systems rather than a totally separate section.

  2. Please update index.rst to include it in the section starting at line 40 as follows:

.. toctree::
   :caption: Configuration
   :maxdepth: 2
   :hidden:

   configuration/maxmind_license
   configuration/command_line_interface
   configuration/cron_jobs
   configuration/variables
   configuration/settings
   configuration/messenger

This adds the file to the navigation structure.

  1. Please install the Vale linter - it is available for PHPStorm and VSCode (and most other IDEs). This will help to pick up grammar issues as you type and help guide you to fix them. Ask for help if you need it with things like passive->active speech as that is hard to do even for a native speaker (ChatGPT can help sometimes!)

  2. Any settings that relate to Messenger from the UI (if any - I wasn't clear on whether any of this is visible in the user interface) should be added to Configuration > Settings - this is where we run through every setting, option and switch in the configuration screens. So explain what they see in the user interface in that file. Link to messenger.rst for more details on what it actually means.

  3. We have to consider that this is the user documentation, for predominantly marketers. I think we need a basic overview of what people need to know to use Mautic out of the box - what is configured by default and where, what are the default settings in the config (always useful if you screw something up and need to revert back without the undo functionality!) etc. Then we should have an advanced section which dives into more detail.

I've asked some users to take a look and give some feedback - we will help to work collaboratively on a structure for the information that will work for all users!

By processing requests in the background, it ensures that the client receives a response before the completion of the request processing.

**Scalability**
It provides the flexibility to scale your Mautic instance by adding more workers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will marketers know what workers are? I don't know if we have explained that concept at all before.

**Load distribution/control**
It enables you to balance and control the server load effectively.

**Improved client response**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are we referring to here when we are talking about clients? Will marketers understand that?

@escopecz
Copy link
Member

@fedys made an update to @galvani 's PR in mautic/mautic#12648 which simplifies the configuration and still enables the option to overwrite it in config-local.php with more advanced configuration if needed. If that PR is accepted, I'll update these docs accordingly.

@RCheesley
Copy link
Member

Closing in favour of #178

@RCheesley RCheesley closed this Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants