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

Feature/add messages for score updated. #34

Merged

Conversation

javleds
Copy link
Contributor

@javleds javleds commented Sep 16, 2021

This PR is a little bit longer, the main idea was to give a solution for the issue #25, nevertheless, to accomplish that required 2 changes:

  1. Look for a better way to build a message without having breaking changes (More details below).
  2. Restrict self score upgrade.

In addition, and it is open to suggestions, we added the username and the score of the affected users.

The final output is similar to this:
bot

I will append a couple of segments that can be added to the wiki in case this PR gets approved, one for the class created to buid the message and other to replace the custom messages.


AdditionalParameters

Build markdown messages for slack can be a little bit hard since we have to create a complex array structures called AdditionalParameters. In that structure we have to define blocks, sections and fields.

This data structure should be serialized in order to be processed by the BotMan and finally give a readable message on Slack.

In example, in order to send the following message:

    Hello world!
    This is the PhpMxBot
    And I love the PhpMx community.
    
    Hope to keep learning a lot
    With you <3

We have to generate the following structure:

    $additionalParameters = ['blocks' => '[{"type":"section","text":{"type":"plain_text","text":"Hello world!","emoji":true}},{"type":"section","text":{"type":"plain_text","text":"This is the PhpMxBot","emoji":true}},{"type":"section","text":{"type":"plain_text","text":"And I love the PhpMx community","emoji":true}},{"type":"divider"},{"type":"section","text":{"type":"plain_text","text":"Hope to keep learning a lot","emoji":true}},{"type":"section","text":{"type":"mrkdwn","text":"*With you* :heart:"}}]'];

Painful, right?

To make easy the creation of that data structure we propose the
AdditionalParametersBuilder class which builds this complex array using
natural and fluent language.

    class AdditionalParametersBuilder:
    
    static create(): MessageBuilder
    ---------------------------------------
    Returns a self instance to use it fluetly.
    
    addText(string $message, bool $processEmoji): MessageBuilder
    ---------------------------------------
    Add a string to the message in a new row.
    
    addMardown(string $message): MessageBuilder
    ---------------------------------------
    Add a string to the message in a new row. This
    strings allows to pass markdown strings and 
    they will be processed according the Slack 
    Api Message Format
    
    addDivider(): MessageBuilder 
    ---------------------------------------
    Add a divider, usefull to separate or format
    mesages.
    
    build(): array
    ---------------------------------------
    Build the required array structure to be used
    by the BotMan.

In order to build the previous example we have to use the following code block:

$additionalParameters = PhpMx\Builders\AdditionalParametersBuilder::create()
            ->addText('Hello world!')
            ->addText('This is the PhpMxBot')
            ->addText('And I love the PhpMx community')
            ->addDivider()
            ->addText('Hope to keep learning a lot')
            ->addMarkdown('*With you* :heart:')
            ->build();

Replace the custom messages

We have created the config/parameters.yml file with 3 important entries:

  • plus_plus.increased_messages
  • plus_plus.decreased_messages
  • plus_plus.not_allowed_messages

In order to change, add or remove any message we have to edit those array properties. Also we have created 2 placeholders, {user} and {score} which will be replaced for the real username and score of the leaderboard.

In example:

'¿De verdad?, Aún tienes {score} puntos {user}.'

Will be rendered as:

'¿De verdad?, Aún tienes 15 puntos @Javier Ledezma.'

config/parameters.yaml Outdated Show resolved Hide resolved
config/services.yaml Outdated Show resolved Hide resolved
{
private array $blocks = [];

public static function create(): self
Copy link
Member

@gueroverde gueroverde Sep 24, 2021

Choose a reason for hiding this comment

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

We need a private constructor ? in order to avoid two way to create a new instance, I think this pattern is called named constructor, here is a reference it's an old video but i think it's a good video https://www.youtube.com/watch?v=J0SFLG5B3wo :p what do you think i am not sure if the private constructor is needed or not 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice suggestion, but the private constructor is not required, this semantic cosntructor allows the user to create and chain methods immediately.

Instead of:

$builder = new AdditionalParametersBuilder();
$builder->addText($text);
$builder->build($text);

We can directly:

AdditionalParametersBuilder::create()->addText($text)->build();

But this is open since sometimes you need to create complex messages based on conditions, IE:

$builder = new AdditionalParametersBuilder();
$builder->addText($text);

if ($condition) {
   $builder->addText($text);
}

$builder->build();

@gueroverde gueroverde self-requested a review September 24, 2021 22:28
@eruizdechavez
Copy link
Contributor

Very nice work! I'd like to see the message suggestion implemented before we merge this pull request.

@javleds
Copy link
Contributor Author

javleds commented Sep 30, 2021

Very nice work! I'd like to see the message suggestion implemented before we merge this pull request.

Thanks @eruizdechavez , what do you mean with see the implementation?

If it is at code level, we already use it in the PlusPlus conversation to build a complex message:

  1. First of all, we create the builder (line 35).
$replyParameters = new AdditionalParametersBuilder();
  1. If the user try to increase its own score, we add a new message (in md) and also a divider, to split message types (line 43).
$replyParameters->addMarkdown($message)->addDivider();
  1. For each modified score, we add a new part of the text in md as well (line 54).
$replyParameters->addMarkdown($message);
  1. Finally, when we want to reply, we build the message (line 57):
$bot->replyInThread('Points updated!', $replyParameters->build());
                                           ^^

is that what you mean?

@eruizdechavez
Copy link
Contributor

I'd like to see the message suggestion implemented before we merge this pull request.

Thanks @eruizdechavez , what do you mean with see the implementation?

@javleds what I mean I want to see it actually implemented with environment variables, SQLite (as I suggested) or any other alternative someone proposes instead of the config files.

I am also flexible if you think that is extra work that should be done on a follow up pull request.

@javleds javleds mentioned this pull request Sep 30, 2021
This was linked to issues Oct 3, 2021
@javleds javleds force-pushed the feature/add-messages-for-score-updated branch from 3b154d7 to 28501d0 Compare October 5, 2021 03:27
@javleds
Copy link
Contributor Author

javleds commented Oct 5, 2021

The origin of the messages were moved from the parametrs.yaml to a database table.

Copy link
Contributor

@eruizdechavez eruizdechavez left a comment

Choose a reason for hiding this comment

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

Nice job! Thanks! 👏 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update notification messages Restrict self ++
4 participants