Bug Report #3817

_primary_key_value not set when pk value is supplied

Added by Martino di Filippo over 3 years ago. Updated over 3 years ago.

Status:ClosedStart date:03/04/2011
Priority:NormalDue date:
Assignee:Lorenzo Pisani% Done:

0%

Category:-
Target version:Kohana v3.x - v3.1.4
Resolution:fixed Points:

Description

I have an Item model with a primary key which is not autoincrement, but it's instead supplied before saving.
I create the model, fill in the fields (pk included), then save().

Kohana_ORM->create() checks if $data[_primary_key] is set before setting both _object[_primary_key] and _primary_key_value, and since it's already set (because the pk value was manually set before save()), _primary_key_value remains NULL. (source:/classes/kohana/orm.php@af3137ed#L1283)

The Item model is left unusable:

Later, I have to add the Item as a _belongs_to relationship to a Detail model.
The Detail model fails to save(), because the non-nullable field `item_id` is filled with $item->pk(), which is NULL.

I believe _object[_primary_key] and _primary_key_value should be set separately, with _primary_key_value always being updated if it's empty, regardless of the status of $data[_primary_key].


Related issues

Duplicated by ORM - Bug Report #4484: When creating a record with a specified primary key _prim... Closed 03/26/2012

Associated revisions

Revision 9d6530d7
Added by Lorenzo Pisani over 3 years ago

Merge pull request #37 from MartinodF/3817-patch

_primary_key_value not set when pk value is supplied (fixes #3817)

History

#1 Updated by Jeremy Bush over 3 years ago

  • Target version set to v3.0.11

#2 Updated by Jeremy Bush over 3 years ago

  • Target version changed from v3.0.11 to v3.0.12

#3 Updated by Martino di Filippo over 3 years ago

Any updates on this?

If my solution is correct (which I believe it to be, since I use it in more than one production environment without problems), should I open a pull request on GitHub or what?

#4 Updated by Martino di Filippo over 3 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

#5 Updated by Lorenzo Pisani over 3 years ago

I don't see this change anywhere on the github repo, could you link to it?

#6 Updated by Martino di Filippo over 3 years ago

I don't know why it has decided to close this bug report automatically, I didn't do it.
I just issued a pull request, which is here: https://github.com/kohana/orm/pull/37

Also, I didn't know which branch to push to, so I created a 3817-patch branch.
You should decide where to merge it eventually :)

#7 Updated by Lorenzo Pisani over 3 years ago

  • Status changed from Closed to Assigned
  • Assignee set to Lorenzo Pisani
  • % Done changed from 100 to 0

Very weird... it looks like GitHub shows all the commits from pull requests as well. Which means Redmine got that commit and thought it was in the main repo, which closed the ticket based on the commit message. Quite awkward. In the future, let's try to use "refs #1234" when you are supplying a fix, the main devs can then use the "fixes..." commit message in their merge commit. That should list everything nicely here.

Thanks for the pull request, I'll make sure this is pulled in as soon as possible.

#8 Updated by Lorenzo Pisani over 3 years ago

A little further explanation... this is what happened:

https://github.com/kohana/orm/commit/d3d7e8e772ed0699149d540b5f4ccdf7cba29b63

See, you can find your commit in the official kohana/orm repo, although it hasn't been pulled into a branch yet. Redmine found this commit, read the message that said it was a fix for this ticket, and closed the ticket. We probably have to document this somewhere in the docs so it doesn't happen again. No pull requests should ever include the "fixes..." command, just "refs..."

At least that's what I think happened.

#9 Updated by Martino di Filippo over 3 years ago

Very weird indeed! Thanks for the explanation, I'll be sure to use "refs" next time. It's actually my first pull request ever, so I'm not very familiar with them, or with Redmine either.

#10 Updated by Lorenzo Pisani over 3 years ago

  • Target version changed from v3.0.12 to v3.1.4

Will make sure this is merged for the next 3.1.x release. 3.0.x is out of support so you will have to maintain that fix manually if you are still using 3.0.x

#11 Updated by Lorenzo Pisani over 3 years ago

  • Status changed from Assigned to Closed
  • Resolution set to fixed

Also available in: Atom PDF