Trello Integration by Team 3 members

[isiddharthsingh]

Teammates:
Siddharth Singh - sms10221@nyu.edu
Adittya Mittal - am14079@nyu.edu

We implemented functionality that allows users to add tasks to their Trello 'To Do' list directly through Slack by simply typing a command (e.g., 'add "study at 5" to my todo'). Additionally, the bot retrieves and displays the current tasks from Trello within Slack, ensuring seamless task management across both platforms."

This version highlights the added functionality and emphasizes the integration between Slack and Trello.

[cindyke124]

Can add a docstring to explain the function’s purpose, input parameters, and return value. This will improve readability and make it easier for other developers to understand its purpose.
For example:
"""Adds a card with the specified name to a predefined Trello list.

Args:
    card_name (str): The name of the card to add.

Returns:
    response.json(): The JSON response containing the details of the added card.

Raises:
    Exception: If the request to Trello fails.
"""

[Sidhved]

The add_card_to_trello function should validate card_name before making the API call.

[cindyke124]

adding type hints can improve clarity regarding the expected input and output types.
For example:
def post_to_slack(channel: str, message: str) -> dict:

[Sidhved]

The error message is directly exposed to users. Consider sanitizing error messages for production.

[AM-git-hub]

Thanks for pointing that out!
Did you mean to mark a different line of code?

[Sidhved]

1. Change Granularity

• Observations: The changes are well-segmented across multiple files, each serving a specific purpose. The PR introduces a set of changes to integrate Trello with Slack, particularly focusing on the “show me todo” and “add todo” commands. The changes appear adequate. However, some commits could be split further to isolate different aspects of functionality (e.g., command handling vs. API interaction).

2. Code Functionality

• Does the Code Meet Requirements?:
  The code meets some of the basic requirements by adding two Slack commands that interact with Trello. The “show my todo” command fetches tasks, while “add todo” allows new tasks to be added. Both commands appear functional based on the code. We would recommend wrapping Trello functionality as a package.
• Logic Flow:
  The logic is mostly sound, but there are a few areas where flow could be improved (check comments on trello_bot.py)

3. Readability & Structure

• Naming Conventions: Generally good, with clear function names like get_trello_list_id, and add_card_to_trello.
• Modularity: Functions are well-encapsulated with single responsibilities. However, some improvements could be made:
  • Consider separating Trello and Slack operations into different modules
  • Create a configuration module for environment variables

4. Standards Compatibility

• Linting: Consider adding more linting rules and try to fix current linting issues.
• API Exposure: API keys and other sensitive information are handled securely, which is good. Using environment variables is a good practice, and the code seems to follow this approach.

5. Testing

• Coverage: Testing coverage could be expanded. Missing test for Trello module (would recommend: error handling, validation, integration)
• Readability: The current tests, while limited, are easy to understand. Improving test readability by adding comments, using more descriptive names for test cases, or adding API documentation to the defined methods would be helpful.

[kyotov]

Major work left.

• How is each file in your PR connected to the task at hand?
  • You should not have files that are irrelevant to the task.
• Make sure files that should be .gitignore are in .gitignore
  • E.g. .DS_Store
• Make sure secrets are not checked in!
• Make sure there is a clear and separated API!

Address these and I will look again.

[kyotov]

there are client libraries for trello for python -- did you decide not to use them on purpose?