laravel icon indicating copy to clipboard operation
laravel copied to clipboard

Missing include bug

Open bbprojectnet opened this issue 3 years ago • 7 comments

Hi,

I have schema (patients resource) like this:

	public function fields(): array
	{
		return [
                        ...
			HasMany::make('documents')->readOnly(),
			HasOne::make('activated-declaration-document')->type('documents')->readOnly(),
			HasOne::make('primary-declaration-document')->type('documents')->readOnly(),
		];
	}

documents resource also have data morph to relation (name dosen't matter in context of bug, i trying to rename).

When i trying GET /api/patients/id?include=primary-declaration-document.data i get 2 includes resources, one document and one data realation of document (declarations resource) - it's fine.

But... when i trying to GET /api/patients/id?include=primary-declaration-document.data,documents then i got many documents, but none of declarations (data relation of document) - i should also get 1 declaration, like in first example.

bbprojectnet avatar Sep 07 '21 07:09 bbprojectnet

Can you share the relationship fields that are on the Document schema?

No idea off the top of my head what the problem with this is. I'll need to recreate your scenario to see if I can replicate the bug.

lindyhopchris avatar Sep 07 '21 08:09 lindyhopchris

Would also be worth sharing the model classes with the relationships that the JSON:API fields are referring to. This is just so that I can accurately recreate the scenario.

FYI my initial gut feel about this might be that if you have a document that appears in the documents relationship and the primary-declaration-document relationship, then the model will be loaded twice. One instance will be loaded as part of the documents relationship and won't have it's data relationship loaded. The second instance will be loaded as part of the primary-declaration-document relationship and will have its data relationship loaded. However, the encoder probably encounters one of these instances first, treats it as a singleton based on its JSON:API type/id combination, and then only encodes one of the two instances.

lindyhopchris avatar Sep 07 '21 08:09 lindyhopchris

Yes, sure:

	public function fields(): array
	{
		return [
			HashId::make()->alreadyHashed(),
			Str::make('dataType')->readOnly(),
			DateTime::make('createdAt')->sortable()->readOnly(),
			DateTime::make('updatedAt')->sortable()->readOnly(),

			BelongsTo::make('patient')->readOnly(),
			MorphTo::make('data')->types('declarations', '...')->readOnly(),
		];
	}

bbprojectnet avatar Sep 07 '21 08:09 bbprojectnet

Document model:

class Document extends Model
{
	use Concerns\BelongsToPatient;
	use Concerns\HashRouteKey;
	use HasFactory;
	use SoftDeletes;

	protected $with = [
		'data',
		'data.attachment',
	];

	/**
	 * Data relation
	 *
	 * @return \Illuminate\Database\Eloquent\Relations\MorphTo
	 */
	public function data(): MorphTo
	{
		return $this->morphTo();
	}
}

and Declaration with DocumentData abstract:

class Declaration extends DocumentData
{
    ...
}
abstract class DocumentData extends Model
{
	use Concerns\HashRouteKey;

	public $timestamps = false;

	/**
	 * Attachment relation
	 *
	 * @return \Illuminate\Database\Eloquent\Relations\BelongsTo
	 */
	public function attachment(): BelongsTo
	{
		return $this->belongsTo(Attachment::class);
	}

	/**
	 * Document relation
	 *
	 * @return \Illuminate\Database\Eloquent\Relations\MorphOne
	 */
	public function document(): MorphOne
	{
		return $this->morphOne(Document::class, 'data');
	}
}

bbprojectnet avatar Sep 07 '21 08:09 bbprojectnet

$with attribute in Document has not effect on this bug, with or without $with is the same.

bbprojectnet avatar Sep 07 '21 08:09 bbprojectnet

Thanks.

Think the workaround at the moment might be:

GET /api/patients/id?include=primary-declaration-document.data,documents.data

Can you confirm if that works? If it does then my suspicions of the cause are probably right!

lindyhopchris avatar Sep 07 '21 12:09 lindyhopchris

Yes, it's works, I did exactly the same to workaround this problem today morning :)

bbprojectnet avatar Sep 07 '21 12:09 bbprojectnet