CityWebConsultants / Iris

Modular content management and web application framework built with Node.js and MongoDB
http://irisjs.org
Other
9 stars 7 forks source link

Entity /fetch is broken #117

Closed pau1m closed 8 years ago

pau1m commented 8 years ago

Does not check credentials. Does not parse entity as array. Logic around failures is muddled (falls through and attempts to set multiple response headers).

FilipNest commented 8 years ago

Oh noes! I'll have a look at this if someone doesn't get there first. The last time something related was pushed was the hook name changes. Do the automatic tests currently cover entity fetch? If not they should.

Could you add step by step reproduction steps for the three errors above? Is this something that used to work but doesn't any more?

pau1m commented 8 years ago

RE Permissions, I've come to wonder if it ever worked. Can't see anywhere in code that pulls credentials from the query on GET.

Have been using queries along the lines of

/fetch?credentials={"token":"032f9437216f13f4ee36402d0ce0638b","userid":"1"}&queries=[{"field":"eid","operator":"IS","value":234}]&entities=["page"]

pau1m commented 8 years ago

@FilipNest

"Do the automatic tests currently cover entity fetch"

Yup, that's where I discovered they were broken. Not sure if they ever worked as the way it was handled in testing meant that Travis didn't count failure as failure. Only after amending tests to use XML reporting via a grunt plugin did they start failing.

"Could you add step by step reproduction steps"

Using the request listed above ends in failure (when the token is valid).

FilipNest commented 8 years ago

Hi! @pau1m . Entity fetch itself is certainly not broken as I've just tried it externally. It might be something to do with the permissions check. I got a 403 forbidden and then got a 200 when I added a permission for the entity fetch for anonymous. Going to try with an actual userid + token.

Update: Just checked this with and without a valid access token and it also works as expected. There's probably a bug somewhere but it's going to be a very specific use case.

FilipNest commented 8 years ago

Here's an entity fetch for users (server running at localhost:1000 change where needed). You'll need to change iris.credentials and set the relevant "can fetch via api... " permission in the Iris system itself.


<script src="https://cdn.socket.io/socket.io-1.4.5.js"></script>
<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.5.0-rc.0/angular.min.js"></script>
<script src="http://localhost:1000/modules/entity/templates.js"></script>
<script src="http://localhost:1000/modules/angular_live_load/angular_live_load_client.js"></script>

<script>

  iris.server = "http://localhost:1000";

  iris.credentials = {
    userid: 1,
    "token": "bb3ce19fd0f38df18d9cda4f116aa195"
  };

  iris.fetchEntities("users", {

    entities: ["user"],
    sort: {
      "username": "asc"
    },
    limit: 2,
    skip: 0

  })

</script>

<div ng-controller="iris-template" ng-iris-template="users">

  <ul>
    <li ng-repeat="user in users">{{user.username}}</li>

  </ul>

</div>
pau1m commented 8 years ago

Following that through. The example I needed lives in template.js iris.fetchEntities = function (variableName, query) {

pau1m commented 8 years ago

@FilipNest I tried the example given as well as the one in the docs. Both result in a server 500

screen shot 2016-03-21 at 10 30 36

FilipNest commented 8 years ago

@pau1m Do you get anything in your Iris logs? So you're saying the template I put above doesn't work for you? Are you doing anything different to it? It works perfectly for me.

pau1m commented 8 years ago

{"name":"iris","hostname":"iris","pid":11872,"level":50,"msg":"Error on line 407 of /var/www/site/docroot/iris_core/modules/core/entity/entity_fetch.js req.query.entities.forEach is not a function","time":"2016-03-21T10:19:41.527Z","v":0}

Same as I was getting before. The way in which params are passed are not parseable.

When you say works perfectly. You mean you've just tried doing the same thing with the same script on the latest code?

FilipNest commented 8 years ago

Yeah. I'll give it another go but it worked on Friday. Maybe I didn't pull the absolute latest. I've got that now so I'll give it another go. Now I've seen the error line we can fix it.

FilipNest commented 8 years ago

Yup. You're right @pau1m . It breaks now. So it must be a recent change.

pau1m commented 8 years ago

Been the same for weeks. Well, the way I was approaching was the same over the last few weeks. Only just tried out the example set you sent this morning. Had the same issue with credentials. The mechanism for parsing does not think it had an object to parse.

FilipNest commented 8 years ago

Hmm... I certainly tried it on Friday and it worked. Nevermind. Now I've seen the log message it should be easy to fix.

FilipNest commented 8 years ago

It's due to the query string sub parameters not being JSON parsed. It's reading ["entityType"] as a string. We need to loop over them and JSON parse them I suppose. Not sure whether to do this further up the chain or just there. It's probably a side effect of when we moved from req.body to req.query .