Validic / validic

API Wrapper for Validic
https://validic.com
MIT License
0 stars 2 forks source link

Non Synced User apps #45

Open alex-bezek opened 8 years ago

alex-bezek commented 8 years ago

I want to query for all user apps, synced: 0, and expanded: 1 to get the supported devices.

I found I can do this by just calling get_org_apps, passing it a user authentication token, and expanded: 1, but this seems like not the intended use case. Would it be more appropriate for a method get_user_apps that takes a params hash (which could contain the user authentication token as well as any other params like expanded or future supported ones). Then get_user_synced_apps would just call this method and pass in synced: 1 as well.

Maybe something like this (untested)

      def get_user_synced_apps(options = {})
        get_user_apps(options.merge(synced: 1))
      end
      alias :get_synced_apps :get_user_synced_apps

      def get_user_apps(options = {})
        auth_token = options.delete(:authentication_token)
        build_response(
          get_request(
            :apps,
            authentication_token: auth_token,
            options
          )
        )
      end

Thanks, Alex

allcentury commented 8 years ago

Alex,

Thanks! Looking at the method in question I can see that the method is named correctly but doesn't give you the ability to change the sync'd flag.

It makes complete sense to me that you would want to see both sync'd and unsync'd app's for a given user. I think something like

def get_user_apps(options)
  options[:synced] ||= 0
  ...
end

then just have get_user_synced_apps call that as you outlined:

def get_user_synced_apps(options)
  get_user_apps(options.merge(synced: 1))
end

I don't see that our docs cover this query param @jeff-brown-validic (can we get that added?), but testing it locally it seems that if I don't supply a synced param it defaults to all. If that's the case the options[:synced] ||= 0 would be merely for self documentation.

Anthony