Skip to content
This repository was archived by the owner on Sep 28, 2022. It is now read-only.

Conversation

@samniisan
Copy link
Contributor

@samniisan samniisan commented Apr 24, 2017

PR has been changed to remove to remove the replaceIfExists option.
fixes: https://github.com/kuzzleio/kuzzle-sdk/issues/21

stafyniaksacha
stafyniaksacha previously approved these changes Apr 24, 2017
@codecov-io
Copy link

codecov-io commented Apr 24, 2017

Codecov Report

Merging #62 into 4.x will increase coverage by 0.35%.
The diff coverage is 94%.

Impacted file tree graph

@@            Coverage Diff            @@
##             4.x      #62      +/-   ##
=========================================
+ Coverage   88.7%   89.05%   +0.35%     
=========================================
  Files         15       15              
  Lines       1204     1252      +48     
=========================================
+ Hits        1068     1115      +47     
- Misses       136      137       +1
Impacted Files Coverage Δ
src/Security/Document.php 95.55% <ø> (+4.44%) ⬆️
src/Security/Security.php 96.95% <100%> (+0.12%) ⬆️
src/Security/User.php 91.02% <92.5%> (+1.55%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6605e07...0412499. Read the comment docs.

@samniisan samniisan self-assigned this Apr 24, 2017
benoitvidis
benoitvidis previously approved these changes Apr 24, 2017
dbengsch
dbengsch previously approved these changes Apr 25, 2017

if (array_key_exists('replaceIfExist', $options)) {
$action = 'createOrReplaceUser';
$fetchResponse = $this->kuzzle->query(
Copy link

@ballinette ballinette Apr 28, 2017

Choose a reason for hiding this comment

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

Suggestion for a cleaner code: replace lines 125-147 by following:

if ($options['replaceIfExist']) {
  $fetchResponse = $this->kuzzle->query(
    $this->buildQueryArgs('getUser'),
    ['_id' => $id]
  );

  if ($fetchResponse["error"] === null && $fetchResponse["result"]["_id"] === $id) {
    $action = 'replaceUser';
  }
}

$response = $this->kuzzle->query(
  $this->buildQueryArgs($action),
  $data,
  $options
);

@dbengsch
Copy link
Contributor

Blocking: ReplaceIfExists Does not match with the new auth refactor

@dbengsch dbengsch assigned dbengsch and unassigned samniisan May 10, 2017
@dbengsch dbengsch changed the title Adds replaceUser route + createUser adaptations Adds replaceUser route and remove replaceIfExists option in createUser May 10, 2017
@dbengsch dbengsch dismissed stale reviews from stafyniaksacha, benoitvidis, ballinette, and themself May 10, 2017 14:06

The PR has changed a lot

@dbengsch dbengsch changed the base branch from 3.x to 4.x May 10, 2017 14:08
@ballinette
Copy link

ballinette commented May 10, 2017

hmmm. I validated, but... why do we remove the replaceIfExists option ? it should be consistent with other SDKs...

I guess it should be implemented as for Javascript SDK : kuzzleio/sdk-javascript#208

@dbengsch
Copy link
Contributor

We are removing it from all SDKs.

@xbill82 xbill82 merged commit df7e7d0 into 4.x May 16, 2017
@xbill82 xbill82 deleted the 21-replace-user branch May 16, 2017 14:11
@scottinet scottinet mentioned this pull request Jun 22, 2017
@scottinet scottinet mentioned this pull request Aug 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants