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

Make users.incremental() work like tickets.incremental() #576

Closed
wants to merge 3 commits into from

Conversation

kovaacs
Copy link
Contributor

@kovaacs kovaacs commented Sep 8, 2023

Fixes #572

Disclaimer: I'm not familiar with Zendesk, I'm working on my first integration right now during which I encountered the issue. This PR does seem to fix it, though.

@cryptomail
Copy link
Collaborator

ok this is SUUUUUPER awesome!
Lemme test this and bat it around today at some time.
Also color me super impressed on the turnaround time <3
I'll create some tests, and ensure it's part of the suite before we just administratively merge it.
Again, THANK YOU SO MUICH.

@@ -706,6 +706,8 @@ class EndpointFactory(object):
users.deleted = PrimaryEndpoint("deleted_users")
users.groups = SecondaryEndpoint('users/%(id)s/groups.json')
users.incremental = IncrementalEndpoint('incremental/users.json')
users.incremental.cursor_start = IncrementalEndpoint(
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmmm yes seems parity was not here :)

@@ -706,6 +706,71 @@ def incremental(self, start_time, include=None, per_page=None):
per_page=per_page)


class IncrementalCursorApi(IncrementalApi):
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice lift

@cryptomail
Copy link
Collaborator

Currently manually testing. Tests pass as-is right now, but I'd like to at least run a few scripts before merge and publish.

@cryptomail
Copy link
Collaborator

Do you have some test cases you're running now?


.. code-block:: python

for ticket in reversed(zenpy_client.tickets.incremental(cursor='xxx')):
Copy link
Collaborator

Choose a reason for hiding this comment

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

you did not write this, but this example face plants :)

for ticket in zenpy_client.tickets.incremental(cursor="MTY4NDYzNDU0Mi4wfHw2fA=="):
	print(ticket)
jteitelbaum@Q7T6X1LF2M pythontests % python3 incremental_users_test.py                     
Traceback (most recent call last):
  File "/Users/jteitelbaum/Code/pythontests/incremental_users_test.py", line 29, in <module>
    for ticket in zenpy_client.tickets.incremental(cursor="MTY4NDYzNDU0Mi4wfHw2fA=="):
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jteitelbaum/Code/zenpy/zenpy/lib/api.py", line 770, in incremental
    raise ValueError(
ValueError: Can't set cursor param and paginate_by_time=True


.. code-block:: python

for ticket in reversed(zenpy_client.tickets.incremental(cursor='xxx')):
Copy link
Collaborator

Choose a reason for hiding this comment

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

perpetuated badness :)

@cryptomail
Copy link
Collaborator

Administratively closed due to issue being resolved with superceding PR

@cryptomail cryptomail closed this Sep 14, 2023
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.

users.incremental() doesn't behave like tickets.incremental()
2 participants