Skip to content

Conversation

mcitdev
Copy link
Contributor

@mcitdev mcitdev commented Jul 2, 2018

Description

Refactoring the verifyUserRelations function to :

  • fix the app crash when the "accessTokens" relation is not correctly set up
  • use more accurate condition in the first if statement
  • prevent the redundant test of "hasManyTokens" in the function

See also #3934

@slnode
Copy link

slnode commented Jul 2, 2018

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

@mcitdev mcitdev requested review from fabien and zbarbuto as code owners July 2, 2018 19:01
@bajtos
Copy link
Member

bajtos commented Jul 3, 2018

@slnode test please

The fix introduced in previous commit was relying on the default
merge algorighm applied when a child model inherits relation config
from the parent.

This commit changes the check to use a more reliable approach
based on the relation metadata configured by the child model.
@bajtos
Copy link
Member

bajtos commented Jul 3, 2018

Thank you @mcitdev for the pull request. The code changes looks good 👍 The commit history was a bit of a mess, I cleaned it up for you and rebased your patch on top of the latest master. Let's wait for CI results before landing.

Because you send the PR from your master branch, you will need a bit of cleanup work on your side to make your master branch up to date with our master branch (the upstream).

  1. If you don't have strongloop's repository configured as an upstream remote yet:

    $ git remote add upstream https://github.com/strongloop/loopback
    
  2. Fetch the latest updates from upstream:

    $ git fetch upstream
    
  3. Hard-reset your master to upstream:

    $ git checkout master
    $ git reset --hard upstream/master
    
  4. Force-push your changes back to GitHub

    $ git push -f
    

In the future, please create a new feature branch for each pull request. I recommend the following guide for step-by-step instructions showing the recommended contribution workflow: https://www.nearform.com/blog/first-time-with-open-source/

@bajtos bajtos changed the title Update the verifyUserRelations function of lib/application.js Make verifyUserRelations() more robust Jul 3, 2018
@bajtos bajtos merged commit 7ad5a2e into strongloop:master Jul 3, 2018
@bajtos
Copy link
Member

bajtos commented Jul 3, 2018

Landed 🎉 Thank you for the contribution ❤️

@bajtos bajtos self-assigned this Jul 3, 2018
@mcitdev
Copy link
Contributor Author

mcitdev commented Jul 3, 2018

I confess that I learned lot about git stuff from you, Thanks @bajtos

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.

3 participants