Skip to content

Set forceId = false for AccessToken - #2501

Merged
jannyHou merged 1 commit into
masterfrom
fix/forceId
Aug 8, 2016
Merged

Set forceId = false for AccessToken#2501
jannyHou merged 1 commit into
masterfrom
fix/forceId

Conversation

@jannyHou

@jannyHoujannyHou commented Jul 13, 2016

Copy link
Copy Markdown
Contributor

Connect to loopbackio/loopback-datasource-juggler#849

Some test cases need refactor after we set forceId as true by default.
Details related please see: loopbackio/loopback-datasource-juggler#982 (comment)

@jannyHou

Copy link
Copy Markdown
ContributorAuthor

@slnode test please

@jannyHoujannyHou self-assigned this Jul 15, 2016
@jannyHou

Copy link
Copy Markdown
ContributorAuthor

@bajtos PTAL, thanks!

@@ -1,5 +1,6 @@
{
"name": "post",
"forceId": false,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AFAICT, we are not setting Post.id explicitly in any of the existing tests. Why is it necessary to set forceId to false in that case?

@bajtos

Copy link
Copy Markdown
Member

@jannyHou I am confused why is it necessary to set forceId:false in test models, since I don't see any test that would be explicitly setting id when creating new instances. It does not match my mental model of how LoopBack & Juggler works.

Could you please elaborate little bit more on why are those changes necessary, so that I can understand what I am missing now?

@bajtos

Copy link
Copy Markdown
Member

@jannyHou let's wait until #2551 is landed, I think we will be able to revert most of the changes proposed here.

@bajtos

Copy link
Copy Markdown
Member

With #2551 landed, there is only one test needing forceId:false. I am going to force-push into this pull request the updated version that works for me. For posterity, @jannyHou's original commit was 8553c2d.

@jannyHou

Copy link
Copy Markdown
ContributorAuthor

@bajtos Thank you 👍 and I will take a look into the CI failings

@jannyHou

Copy link
Copy Markdown
ContributorAuthor

@slnode test please

@jannyHou

jannyHou commented Jul 28, 2016

Copy link
Copy Markdown
ContributorAuthor

The failing ones are caused by authentication problem in some test cases.
For example:

Wed, 27 Jul 2016 21:45:05 GMT strong-error-handler Handling Error: Authorization Required
at C:\jenkins\workspace\loopback\b88abec9\lib\application.js:393:21
at C:\jenkins\workspace\loopback\b88abec9\lib\model.js:318:7
at C:\jenkins\workspace\loopback\b88abec9\common\models\acl.js:471:23
at C:\jenkins\workspace\loopback\b88abec9\node_modules\async\lib\async.js:251:17
at done (C:\jenkins\workspace\loopback\b88abec9\node_modules\async\lib\async.js:132:19)
at C:\jenkins\workspace\loopback\b88abec9\node_modules\async\lib\async.js:32:16
at C:\jenkins\workspace\loopback\b88abec9\node_modules\async\lib\async.js:248:21
at C:\jenkins\workspace\loopback\b88abec9\node_modules\async\lib\async.js:572:34
at C:\jenkins\workspace\loopback\b88abec9\common\models\acl.js:453:17
at C:\jenkins\workspace\loopback\b88abec9\common\models\role.js:269:21
at C:\jenkins\workspace\loopback\b88abec9\node_modules\continuation-local-storage\node_modules\async-listener\glue.js:188:31
at process._tickDomainCallback [as _tickCallback] (node.js:492:13)
<testcase classname="loopback.rest" name="should report 200 for GET /:id/exists found" time="0.007"/>
Wed, 27 Jul 2016 21:45:05 GMT strong-error-handler Response status 401 data {"statusCode":401,"name":"Error","message":"Authorization 

Will investigate more...

@jannyHou

Copy link
Copy Markdown
ContributorAuthor

@slnode test please

@jannyHou

Copy link
Copy Markdown
ContributorAuthor

I am landing it since all loopback CI are green :squirrel:

@jannyHou
jannyHou merged commit 5fd1766 into masterAug 8, 2016
Sign up for freeto 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.

2 participants

@jannyHou@bajtos