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

Added getting started and authentication page #224

Open
wants to merge 6 commits into
base: 5.x
Choose a base branch
from

Conversation

MutiatBash
Copy link

No description provided.

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 the PR @MutiatBash - I've made a few comments but proper reviewing is blocked by #225 which is required to unblock building locally. I'm also noticing that Vale isn't checking this page correctly so we might need to check that, too.


.. note::

Mautic has an API library available for PHP. You can `find it on GitHub <https://github.com/mautic/api-library>`_.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Mautic has an API library available for PHP. You can `find it on GitHub <https://github.com/mautic/api-library>`_.
Mautic has an API library available for PHP. Check out the :xref:`Mautic API Library`.

This needs converting to a link macro. There is already a link file in /links, you can check for the name and use the xref macro as above.


To get started easily, you can use Mautic's Postman collection:

.. image:: https://run.pstmn.io/button.svg
Copy link
Member

Choose a reason for hiding this comment

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

This is very incomplete right now, I'd personally leave this out until we fill it out a bit.

}
}

.. vale off
Copy link
Member

Choose a reason for hiding this comment

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

Why are you turning off Vale here?

Copy link
Author

@MutiatBash MutiatBash Oct 25, 2024

Choose a reason for hiding this comment

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

Hi @RCheesley ...Thank you very much fo your feedback...it's my first time writing rst or hearing about vale....I pretty much copied the content that was linked in the PR you dropped and just made little changes, so i'm really not sure why it was initially added...hence, the reason i didn't take it off...but i'll look into it.


.. vale off

API Rate limiter cache
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
API Rate limiter cache
API rate limiter cache

Copy link
Member

Choose a reason for hiding this comment

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

Looks like this one isn't yet resolved.


``$version`` is in a semantic versioning format: ``[major].[minor].[patch]``. For example: ``2.4.0``. If you'll try it on the latest GitHub version, the version has ``-dev`` at the end. Like ``2.5.1-dev``.

.. vale off
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to turn Vale off here. Instead, fix the problem :)

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.

@MutiatBash a small Vale fix here, too.

* - Authorization code flow
- This flow is best if you want Users to log in with their own Mautic accounts. All actions taken get registered as if the User performed them in Mautic's UI.
* - Client Credentials flow
- This flow is best for Machine-to-Machine, M2M, communications. For example, in cron jobs that run on at fixed times of the day.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- This flow is best for Machine-to-Machine, M2M, communications. For example, in cron jobs that run on at fixed times of the day.
- This flow is best for Machine-to-Machine, M2M, communications. For example, in Cron jobs that run on at fixed times of the day.

Vale fix

@RCheesley RCheesley added documentation Improvements or additions to documentation pending-feedback labels Oct 24, 2024
@MutiatBash
Copy link
Author

@RCheesley .....I've made the suggested changes, can you review them

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 the great work @MutiatBash - just a few small tweaks to fix then it should be good to merge!



Plain HTTP requests
===================
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
===================
===================
.. vale on

Turn it back on here.

$api = new MauticApi();
$contactsApi = $api->newApi('contacts', $auth, $apiUrl);
$contacts = $contactsApi->getList();

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. vale off

Let's turn off Vale here as we are purposely going against the grammar rules to use a specific term.

The OAuth processes can be tricky. If possible, it's best to use an OAuth library for the language that's used. If you're using PHP, Mautic recommends using the :xref:`Mautic API Library`.

Step one - obtain authorization code
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Member

Choose a reason for hiding this comment

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

These should be ~~~~ rather than ^^^^

You should compare the returned ``state`` against the original to ensure the request wasn't tampered with.

Step two - replace with an access token
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Copy link
Member

Choose a reason for hiding this comment

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

As above.

Please store this data in a secure location and use it to authenticate API requests.

Refreshing tokens
^^^^^^^^^^^^^^^^^
Copy link
Member

Choose a reason for hiding this comment

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

As above


.. warning::

Mautic's API library doesn't have support yet for this flow, but there's an open PR that adds support: https://github.com/mautic/api-library/pull/269
Copy link
Member

Choose a reason for hiding this comment

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

Please convert this to a link macro using the make link command, and commit the link file.

Mautic's API library doesn't have support yet for this flow, but there's an open PR that adds support: https://github.com/mautic/api-library/pull/269


Using plain OAuth2 for the Client Credentials flow
Copy link
Member

Choose a reason for hiding this comment

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

Wrap this in a vale off / vale on, also

}


Authenticating the API Request
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Authenticating the API Request
Authenticating the API request

}
}

Mautic version check
Copy link
Member

Choose a reason for hiding this comment

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

Wrap this one in a vale off / vale on.


.. vale off

API Rate limiter cache
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this one isn't yet resolved.

@RCheesley
Copy link
Member

Hi @MutiatBash give us a shout when you're ready for a re-review - it's helpful if you can mark the conversation resolved once you fix each item. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation hacktoberfest-accepted pending-feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants