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

get_group_subscribers filter params does not work as intended #15

Open
jocelynthode opened this issue May 3, 2023 · 3 comments
Open
Assignees
Labels
invalid This doesn't seem right

Comments

@jocelynthode
Copy link

Hey, the filter param for get_group_subscribers does not work as intended.

the doc specifies to user filter={'status': 'active'} however, the code here: https://github.com/mailerlite/mailerlite-python/blob/main/mailerlite/sdk/groups.py#L143 expects the paramter in the following format:

filter={'filter[status]': 'active'}

I have not checked whether this bugs happens on other functions as well.

@igorhrcek igorhrcek self-assigned this May 24, 2023
@igorhrcek igorhrcek added the invalid This doesn't seem right label May 24, 2023
@ytl-liva
Copy link

ytl-liva commented Dec 4, 2023

hi, has this been updated? does not seem to work to get a specific status type 'inactive it returns default which is active

@codebykyle
Copy link

codebykyle commented Feb 8, 2024

Hello,

This ticket appears to be marked as invalid, but is indeed a valid issue.

Per the Mailerlite documentation here:
https://developers.mailerlite.com/docs/groups.html#list-all-groups

The expected parameter is filter[name]

The documentation in the readme here:
https://github.com/mailerlite/mailerlite-python?tab=readme-ov-file#list-all-groups

specifies that the filter parameter be in the format of:
response = client.groups.list(limit=10, page=1, filter={'name': 'My'}, sort='name')

This code will return all groups, as the filter parameter is ignored.

The list function on groups (as an example) just appends the filter to the query parameters directly, without modification:

    def list(self, **kwargs):
        """
        Lists all groups

        Returns a list of all groups.
        Ref: https://developers.mailerlite.com/docs/groups.html#list-all-groups

        :param **kwargs: You can pass additional arguments - page, limit, sort or to filter by name
        :raises: :class: `TypeError` : Got an unknown argument
        :return: JSON array
        :rtype: dict
        """

        available_params = ["list", "limit", "page", "sort", "filter"]

        params = locals()
        query_params = {}
        for key, val in params["kwargs"].items():
            if key not in available_params:
                raise TypeError("Got an unknown argument '%s'" % key)
            if key == "filter":
                for filter_key, filter_value in val.items():
                    query_params[filter_key] = filter_value
            else:
                query_params[key] = val

        return self.api_client.request("GET", self.base_api_url, query_params).json()

For example:

image

As a suggested fix, the loop in list should be updated to something similar to:

        for key, val in params["kwargs"].items():
            if key not in available_params:
                raise TypeError("Got an unknown argument '%s'" % key)
            if key == "filter":
                for filter_key, filter_value in val.items():
                    query_params['filter[%s]' % filter_key] = filter_value
            else:
                query_params[key] = val

You can also just directly change it in the call, but you will be bypassing all of the checks here. If this is what is intended, the documentation should be updated.

api.groups.list(limit=10, page=1, filter={'filter[name]': group_name}, sort='name')

Which method would you like to use? Updating the documentation, or updating the function? Should any consideration be given to people already using filter[name] instead of name?

@jptreen
Copy link

jptreen commented May 19, 2024

Hey!

I had this issue, but with segments. I think the specific issue here has been closed now, but it's present in segments.

I made a more generalised fix to this type of issue in my PR, getting rid of a lot of duplicate code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

5 participants