cwinters8 / gomap Goto Github PK

In an effort to reduce annoying duplication, tests should start using utils.NewCase instead of constructing instances of utils.Case themselves.

The url argument in the Client.httpRequest method is redundant. Remove the arg and use c.Session.APIURL instead.

Figured out a sensible way to parse emails with json marshal and unmarshal. Replace spaghetti code in Email.Get that painfully parses the email body with an implementation similar to the one in GetEmails.

Either allow passing multiple filters or expand the fields on emails.Filter to filter on specific fields (to, from, subject, body, etc). I think still important to leave a general "Text" field to allow matching any field. The solution may also implement a combination of passing multiple filters and using more fields on emails.Filter to get as much specificity as possible. If using multiple filters, perhaps there would need to be an argument to specify if filters should be treated with AND or OR logic.

Need a function that instantiates an Invocation and assigns it a new UUID for the ID field, which will also need its type updated from string to uuid.UUID.

TestInvocationJSON and
TestQueryJSON are not very useful tests currently, because they don't test the structure of the json after it is marshalled.

TestQueryJSON could probably be removed because there are no custom marshal/unmarshal methods associated.

To improve TestInvocationJSON, refer to TestSetJSON/marshal as an example of validating marshalled json structure.

Currently, an Email has an ID field that never gets populated after a successful request and result. It should have a mechanism to be updated after a query request.

Additionally, the Mailbox struct has no concept of having its own ID reflect the ID on the server. The current uuid ID field should be changed to RequestID and a new string ID field should be added.

Using Invocation for both making method calls and parsing responses is not working well. The methodResults object returned with a response seems to often have different fields than are necessary for a request method.

Right now, this is causing the TestMailbox/draft test to fail with the error:

mailbox_test.go:48: failed to create draft email: failed to send email set request: failed to unmarshal response body: failed to unmarshal args: failed to coerce create value to map. value is nil

because create is not an attribute on the result of the Email/set call. Instead, it looks something like

map[string]interface {} [
	"destroyed": nil,
	"notCreated": nil,
	"notDestroyed": nil,
	"notUpdated": nil,
	"accountId": "u69394015",
	"created": map[string]interface {} [
		"fb0c675a-01e4-4764-8677-bd6a1a3ecb6b": *(*interface {})(0xc0002474c8),
	],
	"newState": "3566",
	"oldState": "3564",
	"updated": nil,
]

Currently thinking that a Result struct that could either:

• grow to include many fields to support different methods
• use generics to make Result have only the necessary fields/values for a given method

The latter would be cleaner, but implementing will be more complex.

Need to create documentation for exported functions and some examples.

a Query method needs to be implemented on Email so emails can be searched. possibly should consider a Query function that returns a slice of emails as an alternative or addition.

The process for getting a mailbox ID by name is entirely too long - need a function in the objects/mailboxes package to abstract away some of the implementation details. Could be something like GetMailboxID(name string) (string, error)

Need to implement some top level functions that are more convenient than importing every inner package needed to construct clients, requests, etc

utils.Failf and utils.Checkf are no longer needed, so they should be replaced and removed.

Replace uses of utils.Failf with t.Fatalf.

Replace uses of utils.Checkf by using a "table driven" (struct driven?) approach. For each test case, append a utils.Case to a slice of cases, then iterate over the slice to test each case.

It would be nice to be able to provide a way to chain multiple method calls together into a single request when possible.

When making requests to Fastmail's API directly, you can do something like

[[ "Email/query",{
  "accountId": "ue150411c",
  "filter": {
    "inMailbox": "fb666a55"
  },
  "sort": [{
    "isAscending": false,
    "property": "receivedAt"
  }],
  "collapseThreads": true,
  "position": 0,
  "limit": 30,
  "calculateTotal": true
}, "0" ],
[ "Email/get", {
  "accountId": "ue150411c",
  "#ids": {
    "resultOf": "0",
    "name": "Email/query",
    "path": "/ids"
  },
  "properties": [
    "threadId"
  ]
}, "1" ]]

(source)

to both reduce the number of required HTTP requests and to be able to pass values between method calls.

Similar to the current implementation of the Result interface, Request could be a non-generic interface. This would have the added benefit of enabling chained requests in a simple/sensible manner.

I don't see a real benefit to having the emails and mailboxes packages under an objects directory, and it's annoying to specify in imports.

GetEmailsCall should be an abstraction for the construction of the requests.Call that is currently being done in GetEmails and Email.Get. That way a consumer of the package can create a call and modify the body as needed.

Currently, a MethodType has to be provided when creating a new Invocation, but this should be able to be inferred from the provided arguments value. Need to figure out a better/cleaner implementation.

In order to support retrieving objects by ID, need to enable Object/get requests for emails and mailboxes.

TestSendRequest/set isn't currently doing any result validation. Need to at least validate that the resulting Set body has an ID in the created field.

In order to support sending an email to multiple recipients, arguments.NewMessage should take in a slice of *arguments.Address for the to argument.

Need a new Submit method on the Email object that sends a draft email to the recipient specified in To. Successful call Submit call should also update Email.Keywords.Draft to false