adonisjs / lucid

AdonisJS SQL ORM. Supports PostgreSQL, MySQL, MSSQL, Redshift, SQLite and many more
https://lucid.adonisjs.com/
MIT License
1.02k stars 189 forks source link

feat: add test for null nested BelongsTo relationship #1009

Open theRealPadster opened 4 months ago

theRealPadster commented 4 months ago

🔗 Linked issue

1008

❓ Type of change

📚 Description

This adds a test for nested belongsTo relationships, and checks for nulls. Right now BaseModel.toObject() throws an error when the model has a null preloaded relationship.

TypeError: Cannot read properties of null (reading 'toObject')
    at /app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1507:93
    at Array.reduce (<anonymous>)
    at Proxy.toObject (/app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1505:56)
    at /app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1507:93
    at Array.reduce (<anonymous>)
    at Proxy.toObject (/app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1505:56)
    at /app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1507:93
    at Array.reduce (<anonymous>)
    at Proxy.toObject (/app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1505:56)
    at new AdObject (/app/app/Libraries/Algolia/classes/AdObject.ts:16:25)

Should I add the fix in this PR as well? It will resolve #1008

📝 Checklist

theRealPadster commented 3 months ago

I think this should be good to go. The test does fail. Would the best practice be to include the fix in this PR or a separate one?