Skip to content

Conversation

@izikaj
Copy link

@izikaj izikaj commented Sep 6, 2023

In order to add new GlobalStorage API support there are such updates:

  • Added Monday::Storage class to work with the new API
  • Updated code style for more readability
  • Eased some dependency versions (Faraday is now allowed to have version 2.x)
  • Removed excessive conversions to JSON string
  • The default behavior of the internal Faraday client is set to parse JSON responses automatically
  • Organized existing tests
  • Added tests to cover Monday::Storage behavior
  • Updated readme.

@izikaj
Copy link
Author

izikaj commented Sep 6, 2023

This update has a breaking change - the Monday::Client API requests are now returning hashes instead of strings (so we don't need to do JSON.parse(JSON.parse(response)) anymore).
So we probably should bump the version to 1.0.0, is it ok, or do you prefer 0.2.0?

@izikaj izikaj marked this pull request as ready for review September 6, 2023 06:03
Copy link
Author

Choose a reason for hiding this comment

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

Do we really need the file in repo?

MONDAY_API_URL = "#{MONDAY_PROTOCOL}://api.#{MONDAY_DOMAIN}/v2".freeze
MONDAY_OAUTH_URL = "#{MONDAY_PROTOCOL}://auth.#{MONDAY_DOMAIN}/oauth2/authorize".freeze
MONDAY_OAUTH_TOKEN_URL = "#{MONDAY_PROTOCOL}://auth.#{MONDAY_DOMAIN}/oauth2/token".freeze
MONDAY_STORAGE_URL = "#{MONDAY_PROTOCOL}://apps-storage.#{MONDAY_DOMAIN}/app_storage_api/v2".freeze
Copy link
Author

Choose a reason for hiding this comment

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

Here is the new API endpoint

#
# @param token [String] The users access token (permanent or short-lived) used for API requests.
# @param connection [Faraday::Connection] An existing Faraday connection to be used for the requests.
def initialize(token: nil, api: nil, conn: nil)
Copy link
Author

Choose a reason for hiding this comment

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

Initialization designed to be the same as for Monday::Client

full_url = "#{api_domain}/#{URI.encode_www_form_component(key)}"
return full_url unless shared

"#{full_url}?shareGlobally=true"
Copy link
Author

Choose a reason for hiding this comment

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

This query string came from the original JS library

Copy link
Author

Choose a reason for hiding this comment

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

this should resolve the documentation issue

client.request :json

client.response :raise_error
client.response :json
Copy link
Author

Choose a reason for hiding this comment

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

We should parse JSON in the middleware, to avoid doing this by ourselves


def api_request(url, conn, data, token)
res = conn.post(url, data, 'Authorization' => token, 'Content-Type' => 'application/json')
res.body
Copy link
Author

Choose a reason for hiding this comment

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

Here we have a valid ruby hash, we do not need to convert it to JSON string again

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.

2 participants