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

hasMany or hasOne doesnt work without getIdAttribute #2753

Open
masterbater opened this issue Mar 7, 2024 · 16 comments
Open

hasMany or hasOne doesnt work without getIdAttribute #2753

masterbater opened this issue Mar 7, 2024 · 16 comments

Comments

@masterbater
Copy link
Contributor

This issue has been around, can you guys provide a documentation how to properly make sure things work in hasMany and hasOne

for now using this to make sure hasMany and hasOne work

  public function getIdAttribute($value = null)
    {
        return $value;
    }
@GromNaN
Copy link
Member

GromNaN commented Mar 7, 2024

Hi @masterbater, we have a test suite that cover relationships, including hasMany and hasOne. Could you be more precise about what is the issue: steps to reproduce, expected result and observed result.

@masterbater
Copy link
Contributor Author

Hi @masterbater, we have a test suite that cover relationships, including hasMany and hasOne. Could you be more precise about what is the issue: steps to reproduce, expected result and observed result.

I found the workaround because of this issue #1902 (comment)

I believe the cause of the issue since I am using an existing db that is saved base on mongoose or prisma, foreign keys also get converted into snake_case instead of camelCase.

This are the gist of my collection

rulesauditlogs
{
  "_id": {
    "$oid": "65e74b683d61600fe666d04f"
  },
  "usersId": {
    "$oid": "5be9ce7b3de6dd77db832950"
  },
  "rulesId": {
    "$oid": "65e74addda6dfa40dd5ff39f"
  },
  "createdAt": {
    "$date": "2024-03-05T16:42:16.962Z"
  },
  "updatedAt": {
    "$date": "2024-03-05T16:42:16.962Z"
  },
  "processed": true,
  "isReviewed": true,
  "reviewedAt": {
    "$date": "2024-03-07T10:52:36.027Z"
  },
  "reviewedTimeSpentSeconds": 0
}
rules
{
  "_id": {
    "$oid": "65e74addda6dfa40dd5ff39f"
  },

  "ruleType": "Adherence",
}

@hans-thomas
Copy link
Contributor

@masterbater They're the same as the Eloquent's hasMany or hasOne. You can pass your custom keys as parameters.

For hasOne:

public function hasOne($related, $foreignKey = null, $localKey = null)

For hasMany:

public function hasMany($related, $foreignKey = null, $localKey = null)

The issue that you referred to, is solved by now and no longer exists. I think you are using an older version of this package. Please specify the version you are using.

@masterbater
Copy link
Contributor Author

@masterbater They're the same as the Eloquent's hasMany or hasOne. You can pass your custom keys as parameters.

For hasOne:

public function hasOne($related, $foreignKey = null, $localKey = null)

For hasMany:

public function hasMany($related, $foreignKey = null, $localKey = null)

The issue that you referred to, is solved by now and no longer exists. I think you are using an older version of this package. Please specify the version you are using.

version 4.1.3, I did it work but needs to add getIdAttribute to each models. I pass in custom keys since it convert camelCase keys into snake_keys

@masterbater
Copy link
Contributor Author

 "name": "laravel/laravel",
    "type": "project",
    "description": "The skeleton application for the Laravel framework.",
    "keywords": [
        "laravel",
        "framework"
    ],
    "license": "MIT",
    "require": {
        "php": "^8.1",
        "guzzlehttp/guzzle": "^7.2",
        "laravel/framework": "^10.10",
        "laravel/octane": "^2.3",
        "laravel/sanctum": "^3.3",
        "laravel/tinker": "^2.8",
        "mongodb/laravel-mongodb": "^4.1",
        "mongodb/mongodb": "^1.17"
    },

@hans-thomas
Copy link
Contributor

Could you please share your models and their relationships?

@masterbater
Copy link
Contributor Author

Could you please share your models and their relationships?

Ill try to provide a fresh laravel project and mongodump file later. I will close this if I cant replicate it thanks for replying appreciate it

@hans-thomas
Copy link
Contributor

No sweat buddy❤️

@masterbater
Copy link
Contributor Author

Hi @masterbater, we have a test suite that cover relationships, including hasMany and hasOne. Could you be more precise about what is the issue: steps to reproduce, expected result and observed result.

I think you dont have a test for this. For data consistency we always save the real objectid not a string representation on the collection.

This Fails hasMany

   $user = new User;
    $user->name = 'Test';
    $user->email = 'test@gmail';
    $user->save();
    $readings = new Reading;
    $readings->patientId = new ObjectId($user->id);
    $readings->value = 120;
    $readings->unit = 'mmHg';
    $readings->save();
    return $user->readings;

This make the relationship hasMany work

   $user = new User;
    $user->name = 'Test';
    $user->email = 'test@gmail';
    $user->save();
    $readings = new Reading;
    $readings->patientId = $user->id;
    $readings->value = 120;
    $readings->unit = 'mmHg';
    $readings->save();
    return $user->readings;

User.php

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Notifications\Notifiable;
use MongoDB\Laravel\Eloquent\Model;

class User extends Model
{
    use HasFactory, Notifiable;
    protected $connection = 'mongodb';
    protected $collection = 'users';
    const CREATED_AT = 'createdAt';
    const UPDATED_AT = 'updatedAt';
    public static $snakeAttributes = false;

    public function readings()
    {
        return $this->hasMany(Reading::class, 'patientId');
    }
}

Hopefully it could be fix, this I think should be important especially for some people having different codebases.

@florianJacques
Copy link
Contributor

I don't understand why there is a default attribute mutator on the id key. 👨‍💻

@hans-thomas
Copy link
Contributor

Sorry for the delay. I will work on this whenever I find some free time.

@masterbater
Copy link
Contributor Author

Hi @masterbater, we have a test suite that cover relationships, including hasMany and hasOne. Could you be more precise about what is the issue: steps to reproduce, expected result and observed result.

I think you dont have a test for this. For data consistency we always save the real objectid not a string representation on the collection.

This Fails hasMany

   $user = new User;
    $user->name = 'Test';
    $user->email = 'test@gmail';
    $user->save();
    $readings = new Reading;
    $readings->patientId = new ObjectId($user->id);
    $readings->value = 120;
    $readings->unit = 'mmHg';
    $readings->save();
    return $user->readings;

This make the relationship hasMany work

   $user = new User;
    $user->name = 'Test';
    $user->email = 'test@gmail';
    $user->save();
    $readings = new Reading;
    $readings->patientId = $user->id;
    $readings->value = 120;
    $readings->unit = 'mmHg';
    $readings->save();
    return $user->readings;

User.php

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Factories\HasFactory;
use Illuminate\Notifications\Notifiable;
use MongoDB\Laravel\Eloquent\Model;

class User extends Model
{
    use HasFactory, Notifiable;
    protected $connection = 'mongodb';
    protected $collection = 'users';
    const CREATED_AT = 'createdAt';
    const UPDATED_AT = 'updatedAt';
    public static $snakeAttributes = false;

    public function readings()
    {
        return $this->hasMany(Reading::class, 'patientId');
    }
}

Hopefully it could be fix, this I think should be important especially for some people having different codebases.

@GromNaN Sorry for pinging you directly is there any chance you can support relationship that is natively an objectid? Was this already been resolved?

@GromNaN
Copy link
Member

GromNaN commented Jun 1, 2024

I agree that using ObjectIds is a good practice for foreign keys.
I tried to implement it, but found issues with how the parent Eloquent classes work with foreign keys. They assume every primary/foreign key can be represented as a string. This need in-deep investigation to find how we can implement it in a way that we can maintain in a long-term, without breaking existing application that rely on string foreign keys.

@florianJacques
Copy link
Contributor

Hello, do we have any news on this subject?
Overriding getIdAttribute works most of the time, but breaks belongToMany relationships.

@GromNaN
Copy link
Member

GromNaN commented Jun 28, 2024

There is no plan for now.

@BenasPaulikas
Copy link

Hello, do we have any news on this subject? Overriding getIdAttribute works most of the time, but breaks belongToMany relationships.

I'm also overriding getIdAttribute() and on top of that I do this in model:

    public function getKey(): string
    {
        return (string) parent::getKey();
    }

I do this for some relationships when doing $model->refresh().
This way it reads array key by string and not by ObjectId() which is impossible.

Maybe this could also help you

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

No branches or pull requests

5 participants