freshworks / freshsales-ruby-sdk

A Ruby sdk for Freshsales
MIT License
5 stars 9 forks source link

App token and URL is not encoded #1

Closed Xypus closed 6 years ago

Xypus commented 6 years ago

The way you parse the fs_analytics_config.yml file makes it impossible to use environment variables to store the credentials. The quick fix would be switching the configure_with_yaml method as follows:

    def configure_with_yaml(path_to_yaml_file)
      begin
        parsed_erb = ERB.new File.new(path_to_yaml_file).read
        config     = YAML.load parsed_erb.result(binding)
      rescue Errno::ENOENT
        raise Exceptions.new, 'YAML configuration file could not be found.'
      rescue Psych::SyntaxError
        raise Exceptions.new, 'YAML configuration file contains invalid syntax.'
      end
      configure(config)
    end

This way the credentials could be stored in environment variables and passed to the application like this:

app_token: <%= ENV['FRESHSALES_APP_TOKEN'] %>
url: <%= ENV['FRESHSALES_URL'] %>

With the current approach the credentials have to be submitted to the repository of any application that uses your library.

rmorlok commented 6 years ago

I'd like to double up on this request for similar reasons. I'm going to be copying the code for this library into my application because I don't want to have a separate config file for it, but rather include the configuration information in other environment-specific configuration data.

In addition to @Xypus's suggestion, I would make configure_with_yaml(...) and configure(...) methods public so that the consumer could choose their method of configuration, and then only fall back to the default YAML file format if the library is used without being configured previously.

BhagirathGoud commented 6 years ago

@rmorlok - We have addressed the two issues that you and @Xypus mentioned in v0.1.0 @Xypus - Thank you for the code.

rmorlok commented 6 years ago

@BhagirathGoud thank you!