1

Working on a 'legacy' Lavaral project that has some database structure issues.

There is a pivot table that links services to a user. The issue is that there is no primary key on the table. Instead it has the following:

id (relates to service_id)
user_id
duration
price

The issue is that when updating a single users services information it is actually updating every record in the database where the id (service_id) is the same.

An example: 2 users have the following in the database

id = 1, user_id = 1, duration = null, price = null
id = 1, user_id = 2, duration = null, price = null

The service id of 1 references the ID of a record within the services table.

There is a modal that does an API call for a user to update a specific service, this endpoint (condensed) looks like this:

foreach ($request->services as $key => $value) {
    $serviceRecord = ServiceUser::where('user_id', $userId)->where('id', $value['id'])->first();

    $serviceRecord->duration = ($value['duration'] ? $value['duration'] : null);
    $serviceRecord->price = ($value['price'] ? $value['price'] : null);
    $serviceRecord->save();
}

This currently is updating both of the users records with the same price / duration information.

My issue is that this is now quite a large project and there are countless references to the ServiceUser's id field. Is there a way to make this save only update the record that is returned when doing the first() method? Or do I need to go through the whole app and change it so it uses a primary key for this table?

Lovelock
  • 7,689
  • 19
  • 86
  • 186
  • Could you [edit] in the Eloquent model you have for the table? I suspect the change you'll need to make is there. – IMSoP Oct 25 '17 at 09:35
  • You may find some useful information in these search results: https://stackoverflow.com/search?q=eloquent+composite+primary+key In particular, https://stackoverflow.com/questions/43086722/composite-primary-key-with-eloquent mentions the same protected method as Propaganistas's answer, and https://stackoverflow.com/a/26592507/157957 mentions a different way of approaching the problem. – IMSoP Oct 25 '17 at 12:01
  • Two additional points: Firstly, I'm assuming that your table has no more than one row for a particular combination of `id` and `user_id`, so that those two columns combined can be considered the primary key. Secondly, if you add a new surrogate key (e.g. an auto-increment column) you should be able to tell Eloquent to use that as the primary key and won't need to change how you select rows. – IMSoP Oct 26 '17 at 08:49

1 Answers1

1

The culprit in Laravel is this method in Illuminate\Database\Eloquent\Model:

/**
 * Set the keys for a save update query.
 *
 * @param  \Illuminate\Database\Eloquent\Builder  $query
 * @return \Illuminate\Database\Eloquent\Builder
 */
protected function setKeysForSaveQuery(Builder $query)
{
    $query->where($this->getKeyName(), '=', $this->getKeyForSaveQuery());
    return $query;
}

This basically scopes a save query to the model's primary key. However, since your model doesn't have any unique key defined, it updates all applicable records accordingly, even when you're working with just one model in the variable.

Since there's no real way to work around this Eloquent-coupled behavior, I suggest you just take the "plain old" way of making database calls:

$serviceUser = new ServiceUser;

DB::table($serviceUser->getTable())
  ->where('user_id', $userId)
  ->where('id', $value['id'])
  ->limit(1) // This limits to only the first encountered row
  ->update([
      'duration' => ($value['duration'] ? $value['duration'] : null),
      'price' => ($value['price'] ? $value['price'] : null),
  ]);

Good luck!

(Disclaimer: I didn't test the code, so it might be subject to typos; the gist matters)

Propaganistas
  • 1,674
  • 1
  • 17
  • 40
  • Since this is a `protected` method, would the clean solution not be to over-ride this method on the particular `Model` definition to generate an appropriate `where` clause? – IMSoP Oct 25 '17 at 11:59
  • Since there's no uniqueness between rows, there's no real way of scoping the `setKeysForSaveQuery()` call to only the record it is called from with 100% guarantee. So I think it's mainly a matter of a developer's expectations: calling `save()` on a single model record is expected to only update that record, and not "hoping" it will update the correct one. – Propaganistas Oct 26 '17 at 06:42
  • My assumption (which I now see isn't made explicit in the question) was that the table was unique on `(id, user_id)`, so effectively we have a 2-column primary key. If so, it would be easy to override the query as shown here: https://stackoverflow.com/questions/43086722/composite-primary-key-with-eloquent – IMSoP Oct 26 '17 at 08:40