laravel-enso/versioning

Prevents update conflicts in Laravel

versioning's Issues

Error when saving a versioned model through an associate operation

This is a bug.

Prerequisites

• Are you running the latest version?
• Are you reporting to the correct repository?
• Did you check the documentation?
• Did you perform a cursory search?

Description

Updating a versionable model causes for an error because he wants to persist the version into the database

Steps to Reproduce

Consider the following code

class MyController {

    public function SomeAction(Request $request,MyModel $myModel) {

        $someRelatedModel = SomeRelatedModel::find(1);

        // setting the version attribute from the client to trigger Versionable checks
        $item->version = $request->get('version');

        $item->relation()->associate($someRelatedModel);
        $item->save();

    }

}

This returns the following error:

Column not found: 1054 Unknown column 'version' in 'field list' (SQL: update `items` set `relation_id` = 1, `version` = ?, `items`.`updated_at` = 2019-08-21 15:12:40 where `id` = 1)

Expected behavior

Should correctly save the `$item``

Actual behavior

Error is thrown. I believe because you are adding version to the $this->attributes in the model. I don't think Eloquent likes it...

Feature: Use polymorphism?

Sometime I may just want versioning data to single table so that it will be data driven approach.

Benefits:

1. Can easily enable or disable Versioning based on whether model uses trait or not
2. Helps to clean many tables as every table is free from version column

Down Side:

1. One more query

Being able to temporarily disable the trait

This is a feature request.

Prerequisites

• Are you running the latest version?
• Are you reporting to the correct repository?
• Did you check the documentation?
• Did you perform a cursory search?

Description

Hi,

In our existing migrations, we have some data that we create, for instance the "default user" account. Since we now have added the trait to the User model, the trait comes in actions, and wants to save the versionings table, which at this time does not exist yet, because the migration for the laravel-enso/versioning comes later in the migrations.

Hence, doing a migrate:fresh now basically does no longer work because it crashes when creating models and saving them to the database.

I would like a way to temporarily disable the trait in the database migrations. Would this be a nice feature?

SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'versionable_id' cannot be null (SQL: insert into `versionings` (`version`, `versionable_id`, `versionable_type`, `updated_at`, `created_at`) values (1, , App\\Models\\ModelName, 2020-01-22 05:37:34, 2020-01-22 05:37:34))

I am trying to use this package to apply optimistic locking but I face this problem when I trying to retrieve a model that uses Versionable trait.

ModelName::where('id', $value)->select('version_id', 'name')->first();

Note: this model has another column called "version_id" other than the "version" column

Trace:

{
    "file": "$PROJECT_NAME/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php",
    "line": 252,
    "function": "save",
    "class": "Illuminate\\Database\\Eloquent\\Model",
    "type": "->"
},
{
    "file": "$PROJECT_NAME/vendor/laravel-enso/versioning/src/app/Traits/Versionable.php",
    "line": 94,
    "function": "save",
    "class": "Illuminate\\Database\\Eloquent\\Relations\\HasOneOrMany",
    "type": "->"
},
{
    "file": "$PROJECT_NAME/vendor/laravel-enso/versioning/src/app/Traits/Versionable.php",
    "line": 22,
    "function": "startVersioning",
    "class": "App\\Models\\Listings",
    "type": "->"
}

SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'versionable_id' cannot be null (SQL: insert into `versionings` (`version`, `versionable_id`, `versionable_type`, `updated_at`, `created_at`) values (1, , App\\Models\\ModelName, 2020-01-22 05:37:34, 2020-01-22 05:37:34))

This is a bug.

Prerequisites

• Are you running the latest version?
• Are you reporting to the correct repository?
• Did you check the documentation?
• Did you perform a cursory search?

Description

I am trying to use this package to apply optimistic locking but I face this problem when I trying to retrieve a model that uses Versionable trait.

ModelName::where('id', $value)->select('version_id', 'name')->first();

Trace:

{
    "file": "$PROJECT_NAME/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php",
    "line": 252,
    "function": "save",
    "class": "Illuminate\\Database\\Eloquent\\Model",
    "type": "->"
},
{
    "file": "$PROJECT_NAME/vendor/laravel-enso/versioning/src/app/Traits/Versionable.php",
    "line": 94,
    "function": "save",
    "class": "Illuminate\\Database\\Eloquent\\Relations\\HasOneOrMany",
    "type": "->"
},
{
    "file": "$PROJECT_NAME/vendor/laravel-enso/versioning/src/app/Traits/Versionable.php",
    "line": 22,
    "function": "startVersioning",
    "class": "App\\Models\\Listings",
    "type": "->"
}

Steps to Reproduce

1. Install Laravel.
2. Install the package.
3. Create a model and use the Versionable trait in it.
4. Run a query like this ModelName::where('id', $value)->select('version_id', 'name')->first(); selecting some fields.
5. It will throw an exception.

Note: When you remove the Versionable trait from the model it returns the expected result without exceptions.

Expected behavior

It should returns:

{
  "version_id": "2",
  "name": "hamada"
}

Actual behavior

SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'versionable_id' cannot be null (SQL: insert into `versionings` (`version`, `versionable_id`, `versionable_type`, `updated_at`, `created_at`) values (1, , App\\Models\\ModelName, 2020-01-22 05:37:34, 2020-01-22 05:37:34))

Trait complains about inserting an empty id which he shouldn't be inserting

This is a bug.

Prerequisites

• Are you running the latest version? --> version 1.2.6
• Are you reporting to the correct repository?
• Did you check the documentation?
• Did you perform a cursory search?

Description

Sorry to spam you, but I'm running into yet another strange issue.
This is the code that we are running:

PendingCharge::firstOrCreate([])
    ->invoice_elements()
    ->value(DB::raw("SUM(amount + tax_amount)"));

What it does, is that it requires an object of model PendingCharge(it's globally scoped, so you only get one for the current tenant).
If it doesn't exist, it creates one (firstOrCreate).

Then for that object, it fetches the QueryBuilder for the relation invoice_elements.

We need the sum of that collection for two fields combined.

Now for some reason, this is raising the error

SQLSTATE[23000]: Integrity constraint violation: 1048 **Column 'versionable_id' cannot be null** (SQL: insert into `versionings` (`version`, `versionable_id`, `versionable_type`, `updated_at`, `created_at`) values (1, ?, invoice_element, 2019-08-24 23:54:42, 2019-08-24 23:54:42))

It seems it wants to start versioning on an element that is not currently persisted in the database (invoice_element)

It must have something to do with the ->value() method already emitting an event that you hook into that I didn't expect to be honest.

I currently worked around this by doing two calls to take ->sum('amount') and ->sum('tax_amount') seperately so we can return them, but thought I'd raise this anyhow, so you have track of what we run into :-)

thanks

Steps to Reproduce

See Description

Expected behavior

Not having the error

Actual behavior

An exception

This trait is causing an N+1 query issue

This is a bug.

Prerequisites

• Are you running the latest version?
• Are you reporting to the correct repository?
• Did you check the documentation?
• Did you perform a cursory search?

Description

Hi,

Please check laravel/framework#29630 for detailed explanation. I think it should be fixed in laravel/framework as well, but the way this trait is being set up, causes for an N+1 query issue for each model we apply this to.

I might dig in and submit a PR to remedy this, but I do believe this to be an issue with the trait to be honest.

I'm not sure why you are not adding a getVersionAttribute on the models and make eager loading a requirement, so you can benefit from eager loading, but don't have to set a "virtual property" on the model itself in the retrieved listener...

Versioning field type

This is a question.

Hi. What type should be the versioning field? String? Json?

Best

Laravel 10 support

This is a feature request.

Prerequisites

• Are you running the latest version?
• Are you reporting to the correct repository?
• Did you check the documentation?
• Did you perform a cursory search?

Description

Do you plan to offer Laravel 10 support?
It seems Laravel 10 is very similar to Laravel 9 and there might very few changes needed.
Thanks

Any plans to add Laravel 9 Support?

This is a feature request.

• Are you running the latest version?
• Are you reporting to the correct repository?
• Did you check the documentation?
• Did you perform a cursory search?

Description

Any plans to add support for Laravel 9? (No problem if not, just would be useful to know going forwards).

Steps to Reproduce

• N/A

Expected behavior

• N/A

Actual behavior

• N/A

Compatibility request

feature request**.

Prerequisites

• Are you running the latest version?
• [x ] Are you reporting to the correct repository?
• [ x] Did you check the documentation?
• [ x] Did you perform a cursory search?

Description

Please can we get this package compatible with Laravel 5.1?

Thanks in advance.

arrow functions causing issues.

Hi guys,

You might want to have a quick look at my issue laravel/framework#32159 (comment) as I noticed you are using the new arrow function syntax as well.

When combining two traits that register event handlers to the same eloquent event using arrow functions, you'll run into this issue as well, which directly impacts your packages.

Thought I'd share it with you so you may decide to temporarily revert the upgraded syntax ;-)

Incorrect balance during race condition

This is a bug.

Prerequisites

• [Y] Are you running the latest version?
• [Y] Are you reporting to the correct repository?
• [Y] Did you check the documentation?
• [Y] Did you perform a cursory search?

Description

Referring to following link example:
https://medium.com/@aslrousta/pessimistic-vs-optimistic-locking-in-laravel-264ec0b1ba2
We attempt to create multiple concurrent request to deduct payment from 1 account, after implement the package, the result are same, when there is race condition, the database value are still having wrong calculation.

Steps to Reproduce

Mechanism to implement the package:
composer require laravel-enso/versioning:1.4.3
Composer updated my laravel from 7.12.0 to 7.17.2 automatically
Amend enso migration file to big integer + unsigned
php artisan migrate

In account model:

use LaravelEnso\Versioning\App\Traits\Versionable;

class Account extends Model
{
use Versionable;

Our configurations as below:

1. Apache to handle 600 connections
2. 1 x php artisan serve running
3. 4 x php artisan queue:work
4. jMeter fire 25 threads [users] x 2 loops in 1 second
5. Update account model from another user model

Laravel Version: 7.17.2
Queue System: Redis
Enso/versioning: 1.4.3
Database: MySQL 8.0
OS: Mac OS 10.15.5

We amended your migration file to:
Schema::create('versionings', function (Blueprint $table) {
$table->bigInteger('id')->unsigned();

        $table->string('versionable_type');
        $table->unsignedBigInteger('versionable_id');

        $table->bigInteger('version')->unsigned();

        $table->timestamps();

        $table->unique(['versionable_type', 'versionable_id']);
    });

Unsigned and BigInteger is needed as 1 account will be used forever

Expected behavior

1. versionings table updated_at shows latest time after our test
2. account model balance_amount shall show correct amount in race condition

Actual behavior

1. Wrong balance_amount in account model