Conversation
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.
adarsh
left a comment
There was a problem hiding this comment.
A few minor questions. Looks good to me.
| attribute :folder_id, :string | ||
|
|
||
| has_n_of_attribute :labels, :label | ||
| has_n_of_attribute :label_ids, :string |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
We do have started and unread define above the file so we are good. The label_ids was the missing one.
There was a problem hiding this comment.
| end | ||
|
|
||
| def update(data) | ||
| unupdatable_attributes = data.keys.reject { |name| UPDATABLE_ATTRIBUTES.include?(name) } |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Right now there is no way to set a
labelon amessage.API accepts
label_idsas an array of string of label ids.This PR also add
UPDATABLE_ATTRIBUTESto just allow few attributes tobe updated allowed by API instead of having all attributes to be
updated.