Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

not including foreignKey in relation fields crashes loopback app #890

Closed
ebarault opened this issue Apr 5, 2016 · 5 comments
Closed

not including foreignKey in relation fields crashes loopback app #890

ebarault opened this issue Apr 5, 2016 · 5 comments
Labels

Comments

@ebarault
Copy link
Contributor

ebarault commented Apr 5, 2016

consider the following relation:

        "conversation": {
            "type": "hasOne",
            "model": "Conversation",
            "foreignKey": "channelId",
            "scope": {
                "fields": ["id"]
            }
        }

note that the channelId foreignKey is missing in the fields filter: this gives the following error which just crashes the loopback app

2016-04-05T09:36:49.881Z pid:9557 worker:3 /var/lib/strong-pm/svc/1/work/8c5ceb3bf21d2beeb0c389ce02f87bad80f11ccc.1459848875604/node_modules/loopback-datasource-juggler/lib/include.js:812
2016-04-05T09:36:49.881Z pid:9557 worker:3             var objList = objTargetIdMap[targetId.toString()];
2016-04-05T09:36:49.882Z pid:9557 worker:3                                                  ^
2016-04-05T09:36:49.883Z pid:9557 worker:3 TypeError: Cannot read property 'toString' of undefined
2016-04-05T09:36:49.883Z pid:9557 worker:3     at linkOneToMany (/var/lib/strong-pm/svc/1/work/8c5ceb3bf21d2beeb0c389ce02f87bad80f11ccc.1459848875604/node_modules/loopback-datasource-juggler/lib/include.js:812:50)

adding the foreignKey in the fields filter solves the issue
"fields": ["id", "channelId"]

This is nonsense the app crashes for such reason.
Better than fixing, would you consider forcing the foreignKeys by default in fields filter ?

in my case, the relation

        "conversation": {
            "type": "hasOne",
            "model": "Conversation",
            "foreignKey": "channelId",
            "scope": {
                "fields": ["id"]
            }
        }

would implicitly traduce as a "fields": ["id", "channelId"] filter

@ebarault ebarault changed the title not including foreignKey in relation fields crashes the api not including foreignKey in relation fields crashes loopback app Apr 5, 2016
@superkhau superkhau self-assigned this Apr 5, 2016
@superkhau
Copy link
Contributor

Can you provide a sample repo for me to reproduce the issue on my end?

@ebarault
Copy link
Contributor Author

ebarault commented Apr 5, 2016

hi superkhau, i'll package a sample loopback app and send it through

@superkhau
Copy link
Contributor

You can fork loopback-sandbox with just some dummy data. See https://github.com/strongloop/loopback/wiki/Reporting-issues#bug-report

@ebarault
Copy link
Contributor Author

ebarault commented Apr 6, 2016

see this sample repo to reproduce the issue:

the sample has 2 models: parent and child with a hasOne relation from parent to child

as per parent.json the relation is defaulted to

"scope": {
        "include": [
            "child"
        ]
    },
    "relations": {
        "child": {
            "type": "hasOne",
            "model": "Child",
            "foreignKey": "parentId",
            "scope": {
                "fields": ["id"]
            }
        }
    }

note the missing parentId foreignKey in default fields

i created 2 boot scripts:

  • one to create dummy data
  • the other to run a query that will fail

simply launch node .
the app should crash with the incriminated error

just add the parentId foreignKey in fields included by default to have it work

i tested, it runs exactly the same if i don't include the relation and fields by default and specify them in the query

@superkhau superkhau removed the triaging label Apr 8, 2016
@superkhau superkhau removed their assignment Apr 8, 2016
@0candy 0candy added the bug label May 19, 2016
@jannyHou
Copy link
Contributor

This is a duplicate issue of #779
To keep it easy to track, I will update the fix and move all discussion to that issue :)
I am closing this one, thanks for understanding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants