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.

Sign up for GitHub

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

Allow setting label_ids on a message #231

Merged
merged 7 commits into from
Aug 22, 2019
Merged

Allow setting label_ids on a message #231

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2bd17ce
Allow setting `label_ids` on a message
arunagw Aug 21, 2019
d736143
Fix rubocop
arunagw Aug 21, 2019
950a7ab
Move into private method
arunagw Aug 22, 2019
e40e7ce
Add FilterAttributes class
arunagw Aug 22, 2019
145c1d1
Better error message
arunagw Aug 22, 2019
a320a89
Add spec for FilterAttributes
arunagw Aug 22, 2019
ca7a37e
Fix cop
arunagw Aug 22, 2019
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
Next Next commit
Allow setting label_ids on a message 
Right now there is no way to set a `label` on a `message`.

API accepts `label_ids` as an array of string of label ids.

This PR also add `UPDATABLE_ATTRIBUTES` to just allow few attributes to
be updated allowed by API instead of having all attributes to be
updated.
  • Loading branch information
@arunagw
arunagw committed Aug 21, 2019
commit 2bd17ce7dcf78be015d8e6dff8f70a64eed57ea8
13 changes: 13 additions & 0 deletions lib/nylas/message.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ class Message
has_n_of_attribute :events, :event
has_n_of_attribute :files, :file
attribute :folder, :folder
attribute :folder_id, :string

has_n_of_attribute :labels, :label
has_n_of_attribute :label_ids, :string
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to add starred and unread attributes here also? I'm not familiar with the DSL so I'm just looking at the pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am going to check with these attributes and see what's happening. If we don't need it I will remove them from here. Thanks for the point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do have started and unread define above the file so we are good. The label_ids was the missing one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are actually here https://github.com/nylas/nylas-ruby/pull/231/files#diff-6027b1238ffa1b9852b7b40018edcbabL31-L32


transfer :api, to: %i[events files folder labels]

Expand All @@ -46,6 +49,16 @@ def unread?
unread
end

UPDATABLE_ATTRIBUTES = %i[label_ids folder_id starred unread].freeze
def update(data)
unupdatable_attributes = data.keys.reject { |name| UPDATABLE_ATTRIBUTES.include?(name) }
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about extracting a private method, something named like def unupdatable_attributes? to raise the exception. Then it would maybe look like

def update(data)
  if unupdatable_attributes?(data)
    raise ArgumentError, # message
  else  
    super(data)
  end
end

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a new class to filter attributes here e40e7ce

I think we are going to need this class everywhere where we don't want to send unwanted params to server.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh this is beautiful ✨

unless unupdatable_attributes.empty?
raise ArgumentError, "Cannot update #{unupdatable_attributes} only " \
"#{UPDATABLE_ATTRIBUTES} are updatable"
end
super(data)
end

def expanded
return self unless headers.nil?

Expand Down
35 changes: 35 additions & 0 deletions spec/nylas/message_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,41 @@
end
end

describe "#update" do
it "let's you set the starred, unread, folder, and label ids" do
api = instance_double(Nylas::API, execute: "{}")
message = described_class.from_json('{ "id": "message-1234" }', api: api)

message.update(
starred: true,
unread: false,
folder_id: "folder-1234",
label_ids: %w[label-1234 label-4567]
)

expect(api).to have_received(:execute).with(
method: :put, path: "/messages/message-1234",
payload: JSON.dump(
starred: true, unread: false,
folder_id: "folder-1234",
label_ids: %w[
label-1234
label-4567
]
)
)
end

it "raises an argument error if the data has any keys that aren't allowed to be updated" do
api = instance_double(Nylas::API, execute: "{}")
message = described_class.from_json('{ "id": "message-1234" }', api: api)
expect do
message.update(subject: "A new subject!")
end.to raise_error ArgumentError, "Cannot update [:subject] only " \
"#{described_class::UPDATABLE_ATTRIBUTES} are updatable"
end
end

describe "#expanded" do
it "fetch or return expanded version of message" do
api = instance_double(Nylas::API, execute: "{}")
Expand Down