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

Python client notes and call logs API support #86

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ chartmogul.Customer.connectSubscriptions(config, uuid='cus_5915ee5a-babd-406b-b8
})
chartmogul.Customer.contacts(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb', cursor='aabbcc', per_page=20)
chartmogul.Customer.createContact(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb', data={})
chartmogul.Customer.notes(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb', cursor='aabbcc', per_page=20)
chartmogul.Customer.createrNote(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb', data={})
Copy link

Choose a reason for hiding this comment

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

Suggested change
chartmogul.Customer.createrNote(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb', data={})
chartmogul.Customer.createNote(config, uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb', data={})

```

#### [Contacts](https://dev.chartmogul.com/docs/contacts)
Expand All @@ -161,6 +163,15 @@ chartmogul.Contact.modify(config, uuid='con_5915ee5a-babd-406b-b8ce-d207133fb4cb
chartmogul.Contact.destroy(config, uuid='con_5915ee5a-babd-406b-b8ce-d207133fb4cb')
```

#### [Customer Notes](https://dev.chartmogul.com/docs/customer_notes)
```python
chartmogul.CustomerNote.create(config, data={})
chartmogul.CustomerNote.all(config, cursor='aabbcc', per_page=20, customer_uuid='cus_5915ee5a-babd-406b-b8ce-d207133fb4cb')
chartmogul.CustomerNote.retrieve(config, uuid='note_5915ee5a-babd-406b-b8ce-d207133fb4cb')
chartmogul.CustomerNote.patch(config, uuid='note_5915ee5a-babd-406b-b8ce-d207133fb4cb')
chartmogul.CustomerNote.destroy(config, uuid='note_5915ee5a-babd-406b-b8ce-d207133fb4cb')
```

#### [Customer Attributes](https://dev.chartmogul.com/docs/customer-attributes)

Note that the returned attributes of type date are not parsed and stay in string.
Expand Down Expand Up @@ -256,7 +267,7 @@ chartmogul.Transaction.create(config, uuid='inv_745df1d4-819f-48ee-873d-b5204801
```python
import chartmogul
chartmogul.SubscriptionEvent.all(config)
chartmogul.SubscriptionEvent.create(config, data={
chartmogul.SubscriptionEvent.create(config, data={
'subscription_event' : {
'external_id' : 'evnt_026',
'customer_external_id' : 'scus_022',
Expand Down
1 change: 1 addition & 0 deletions chartmogul/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from .api.attributes import Attributes
from .api.custom_attrs import CustomAttributes
from .api.customer import Customer
from .api.customer_note import CustomerNote
from .api.contact import Contact
from .api.data_source import DataSource
from .api.invoice import Invoice
Expand Down
3 changes: 3 additions & 0 deletions chartmogul/api/customer.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from collections import namedtuple
from .attributes import Attributes
from .contact import Contact
from .customer_note import CustomerNote


class Address(DataObject):
Expand Down Expand Up @@ -82,3 +83,5 @@ def make(self, data, **kwargs):
Customer.createContact = Contact._method(
"create", "post", "/customers/{uuid}/contacts", useCallerClass=True
)
Customer.notes = CustomerNote._method("all", "get", "/customer_notes?customer_uuid={uuid}", useCallerClass=True)
Customer.createNote = CustomerNote._method("create", "post", "/customer_notes", useCallerClass=True)
29 changes: 29 additions & 0 deletions chartmogul/api/customer_note.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
from marshmallow import Schema, fields, post_load, EXCLUDE
from ..resource import Resource
from collections import namedtuple


class CustomerNote(Resource):
"""
https://dev.chartmogul.com/v1.0/reference#customer-notes
"""

_path = "/customer_notes{/uuid}"
_root_key = "entries"
_many = namedtuple("CustomerNotes", [_root_key, "has_more", "cursor"])

class _Schema(Schema):
uuid = fields.String()
customer_uuid = fields.String(allow_none=True)
type = fields.String(allow_none=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we should set allow_none=True for these fields. Customer note should always belong to a customer and the type should always be there

text = fields.String(allow_none=True)
author_email = fields.String(allow_none=True)
call_duration = fields.Int(allow_none=True)
created_at = fields.DateTime(allow_none=True)
updated_at = fields.DateTime(allow_none=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for author_email, created_at and updated_at


@post_load
def make(self, data, **kwargs):
return CustomerNote(**data)

_schema = _Schema(unknown=EXCLUDE)
71 changes: 70 additions & 1 deletion test/api/test_customer.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import unittest
from chartmogul import Customer, Contact, Config
from chartmogul import Customer, Contact, Config, CustomerNote
from chartmogul.api.customer import Attributes, Address
from datetime import datetime
from chartmogul import APIError
Expand Down Expand Up @@ -279,6 +279,35 @@
],
}

note = {
"customer_uuid": "cus_00000000-0000-0000-0000-000000000000",
"type": "note",
"text": "This is a note",
"call_duration": 0,
"author_email": "john@example.com",
"created_at": "2015-06-09T13:16:00-04:00",
"updated_at": "2015-06-09T13:16:00-04:00"
}

createNote = {
"customer_uuid": "cus_00000000-0000-0000-0000-000000000000",
"type": "note",
"text": "This is a note",
"author_email": "john@xample.com"
}

noteEntry = {
"uuid": "cus_00000000-0000-0000-0000-000000000000",
"customer_uuid": "cus_00000000-0000-0000-0000-000000000000",
"type": "note",
"text": "This is a note",
"call_duration": 0,
"author_email": "john@example.com",
"created_at": "2015-06-09T13:16:00-04:00",
"updated_at": "2015-06-09T13:16:00-04:00"
}

allNotes = {"entries": [noteEntry], "cursor": "cursor==", "has_more": True}

class CustomerTestCase(unittest.TestCase):
"""
Expand Down Expand Up @@ -480,3 +509,43 @@ def test_createContact(self, mock_requests):
self.assertEqual(mock_requests.last_request.qs, {})
self.assertEqual(mock_requests.last_request.json(), createContact)
self.assertTrue(isinstance(expected, Contact))

@requests_mock.mock()
def test_notes(self, mock_requests):
mock_requests.register_uri(
"GET",
"https://api.chartmogul.com/v1/customer_notes?customer_uuid=cus_00000000-0000-0000-0000-000000000000",
status_code=200,
json=allNotes,
)

config = Config("token")
notes = Customer.notes(config, uuid="cus_00000000-0000-0000-0000-000000000000").get()
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also test with other params? e.g. per_page, cursor. Reason why I asked is because you defined the URL here:

Customer.notes = CustomerNote._method("all", "get", "/customer_notes?customer_uuid={uuid}", useCallerClass=True)

and the customer_uuid is part of the URL. I'm not sure if the parameters would merge correctly with the rest (i.e. would it look like ?customer_uuid=nn&cursor=nn or ?customer_uuid=nn?cursor=nn), so maybe it's better to test that

expected = Customer._many(**allNotes)

self.assertEqual(mock_requests.call_count, 1, "expected call")
self.assertEqual(mock_requests.last_request.qs, {'customer_uuid': ['cus_00000000-0000-0000-0000-000000000000']})
self.assertEqual(mock_requests.last_request.text, None)
self.assertEqual(sorted(dir(notes)), sorted(dir(expected)))
self.assertTrue(isinstance(notes.entries[0], CustomerNote))
self.assertEqual(notes.cursor, "cursor==")
self.assertTrue(notes.has_more)

@requests_mock.mock()
def test_createNote(self, mock_requests):
mock_requests.register_uri(
"POST",
"https://api.chartmogul.com/v1/customer_notes",
status_code=200,
json=note,
)

config = Config("token")
expected = Customer.createNote(
config, data=createNote
).get()

self.assertEqual(mock_requests.call_count, 1, "expected call")
self.assertEqual(mock_requests.last_request.qs, {})
self.assertEqual(mock_requests.last_request.json(), createNote)
self.assertTrue(isinstance(expected, CustomerNote))
127 changes: 127 additions & 0 deletions test/api/test_customer_note.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
import unittest
from chartmogul import CustomerNote, Config
import requests_mock

from pprint import pprint

note = {
"uuid": "note_00000000-0000-0000-0000-000000000000",
"customer_uuid": "cus_00000000-0000-0000-0000-000000000000",
"type": "note",
"text": "This is a note",
"call_duration": 0,
"author": "John Doe (john@example.com)",
"created_at": "2015-06-09T13:16:00-04:00",
"updated_at": "2015-06-09T13:16:00-04:00"
}

createNote = {
"customer_uuid": "cus_00000000-0000-0000-0000-000000000000",
"type": "note",
"text": "This is a note",
"authoer_email": "john@xample.com"
}

allNotes = {"entries": [note], "cursor": "cursor==", "has_more": False}


class CustomerNoteTestCase(unittest.TestCase):
"""
Tests complex nested structure & assymetric create/retrieve schema.
"""

@requests_mock.mock()
def test_all(self, mock_requests):
mock_requests.register_uri(
"GET",
"https://api.chartmogul.com/v1/customer_notes?cursor=Ym9veWFo&per_page=1&customer_uuid=cus_00000000-0000-0000-0000-000000000000",
status_code=200,
json=allNotes,
)

config = Config("token")
notes = CustomerNote.all(
config,
customer_uuid="cus_00000000-0000-0000-0000-000000000000",
cursor="Ym9veWFo",
per_page=1,
).get()
expected = CustomerNote._many(**allNotes)

self.assertEqual(mock_requests.call_count, 1, "expected call")
self.assertEqual(
mock_requests.last_request.qs,
{
"cursor": ["ym9vewfo"],
"per_page": ["1"],
"customer_uuid": ["cus_00000000-0000-0000-0000-000000000000"],
},
)
self.assertEqual(mock_requests.last_request.text, None)
self.assertEqual(dir(notes), dir(expected))
self.assertTrue(isinstance(notes.entries[0], CustomerNote))
self.assertFalse(notes.has_more)
self.assertEqual(notes.cursor, "cursor==")

@requests_mock.mock()
def test_create(self, mock_requests):
mock_requests.register_uri(
"POST", "https://api.chartmogul.com/v1/customer_notes", status_code=200, json=note
)

config = Config("token")
CustomerNote.create(config, data=createNote).get()
self.assertEqual(mock_requests.call_count, 1, "expected call")
self.assertEqual(mock_requests.last_request.qs, {})
self.assertEqual(mock_requests.last_request.json(), createNote)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also IMO it's better to assert the return value of .create instead of asserting the data payload to confirm that the return value matches the mocked data


@requests_mock.mock()
def test_patch(self, mock_requests):
mock_requests.register_uri(
"PATCH",
"https://api.chartmogul.com/v1/customer_notes/note_00000000-0000-0000-0000-000000000000",
status_code=200,
json=note,
)

config = Config("token")
CustomerNote.patch(
config, uuid="note_00000000-0000-0000-0000-000000000000", data=createNote
).get()
self.assertEqual(mock_requests.call_count, 1, "expected call")
self.assertEqual(mock_requests.last_request.qs, {})
self.assertEqual(mock_requests.last_request.json(), createNote)

@requests_mock.mock()
def test_retrieve(self, mock_requests):
mock_requests.register_uri(
"GET",
"https://api.chartmogul.com/v1/customer_notes/note_00000000-0000-0000-0000-000000000000",
status_code=200,
json=note,
)

config = Config("token")
expected = CustomerNote.retrieve(
config, uuid="note_00000000-0000-0000-0000-000000000000"
).get()

self.assertEqual(mock_requests.call_count, 1, "expected call")
self.assertEqual(mock_requests.last_request.qs, {})
self.assertTrue(isinstance(expected, CustomerNote))

@requests_mock.mock()
def test_destroy(self, mock_requests):
mock_requests.register_uri(
"DELETE",
"https://api.chartmogul.com/v1/customer_notes/note_00000000-0000-0000-0000-000000000000",
status_code=200,
json=note,
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look right; when the note is deleted, it returns {} instead

Suggested change
json=note,
json={},

)

config = Config("token")
CustomerNote.destroy(
config, uuid="note_00000000-0000-0000-0000-000000000000"
).get()
self.assertEqual(mock_requests.call_count, 1, "expected call")
self.assertEqual(mock_requests.last_request.qs, {})
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to assert the return value of the .destroy too for consistency

Loading