Skycatch / node-box

Box API client for Node.JS
MIT License
21 stars 19 forks source link

adding upsert method for folders #3

Open ekump opened 9 years ago

ekump commented 9 years ago

add a method that will attempt to create a folder. If there is already a folder in that location with that name return that folder. Otherwise, create a new one.

lyaunzbe commented 9 years ago

Looks good, will merge soon. I also think I will just rename this method to 'create' and replace the current 'create' method. Thanks for your contribution.

srt32 commented 9 years ago

Ping @lyaunzbe, curious what your thoughts / plans are for this idea of upserting.

srt32 commented 9 years ago

@ekump do you know the status of this PR? I'm looking to implement something similar on a project and I'd enjoy not doing it myself ;)

lyaunzbe commented 9 years ago

@srt32 I will gladly merge this in (it looks good to me),but do you mind just testing @ekump's branch to confirm that it works and leaving a comment here? I'd do it myself but currently don't have the time.

srt32 commented 9 years ago

@lyaunzbe I can test early this week. Would you be open to my taking a stab at a basic test suite as well?

lyaunzbe commented 9 years ago

Of course @srt32 ! Your contributions would be much appreciated.

srt32 commented 9 years ago

Sorry about the delay here. I got swamped at work. I plan to tackle this manual test over the next week.

srt32 commented 8 years ago

I added the start to a test suite in https://github.com/Skycatch/node-box/pull/12. If things look good there, I can add a similar kind of test for this new functionality (which I'm excited to use ;)).

srt32 commented 8 years ago

I manually tested this functionality today and it works but the responses are not uniform. See below for more details.

Steps:

{ type: 'folder',
  id: '4770752700',
  sequence_id: '0',
  etag: '0',
  name: 'test_srt32',
  created_at: '2015-09-28T16:44:45-07:00',
  modified_at: '2015-09-28T16:44:45-07:00',
  description: '',
  size: 0,
  path_collection: { total_count: 2, entries: [ [Object], [Object] ] },
  created_by:
   { type: 'user',
     id: '248346000',
     name: 'user name',
     login: 'user@example.com' },
  modified_by:
   { type: 'user',
     id: '248346000',
     name: 'user name foo',
     login: 'user@example.com' },
  trashed_at: null,
  purged_at: null,
  content_created_at: '2015-09-28T16:44:45-07:00',
  content_modified_at: '2015-09-28T16:44:45-07:00',
  owned_by:
   { type: 'user',
     id: '237410600',
     name: 'user name',
     login: 'user@example.com' },
  shared_link: null,
  folder_upload_email: null,
  parent:
   { type: 'folder',
     id: '4535300400',
     sequence_id: '0',
     etag: '0',
     name: 'tmp' },
  item_status: 'active',
  item_collection:
   { total_count: 0,
     entries: [],
     offset: 0,
     limit: 100,
     order: [ [Object], [Object] ] } }
{ type: 'folder',
  id: '4770752700',
  sequence_id: '0',
  etag: '0',
  name: 'test_srt32' }

The functionality worked but the response from the function will be different depending on which scenario we're in (as the response from Box is different). It seems like, ideally, we'd have the same response from this function regardless of what happens internally. Perhaps we wrap up the response in some way so that it's uniform?

srt32 commented 8 years ago

@ekump @lyaunzbe, curious about your thoughts regarding the return values of this function. Having the idempotent folder creation land would be great but I'm not sure if we want it as is currently.

srt32 commented 8 years ago

Ping @ekump @lyaunzbe, would either of you object to my taking over here and standardizing the response?

ekump commented 8 years ago

@srt32 No objections here.

lyaunzbe commented 8 years ago

No objections, just submit the standardized request to this branch/pr.