Store primary key column name in lower case

[kamrandotpk]

Often, developers set the column names in different cases for the $primary_key static field in the Model. However, the object for the Model stores column names as lowercase (in $attributes). This causes confusion at times (for example, when checking if "$attributes[$pk]" is set while inserting a new record).

This small bit of code makes sure no matter what case the developer sets his primary key ("myPrimaryKey" or "myprimaryKey"), the key will always be stored in all lowercase ("myprimarykey").

[al-the-x]

Now that we have Travis CI working, can you fetch upstream, rebase your branch, and force-push to trigger a rebuild?

[kamrandotpk]

I am not sure if I followed the right steps, but here is what I did:

1. git pull upstream master
2. git rebase origin/master
3. git push origin master --force

Please let me know if I messed up or if its all okay. I am not too good at using Git. Thanks.

[Rican7]

@thesocialgeek
Ouch... you didn't exactly do what @al-the-x was saying. Unfortunately, when you did "git pull upstream master", you merged in all of the changes from upstream. 😟

If you can revert that mistake, that would be great.
What you should have done, was:

1. git checkout master
2. git fetch --all
3. git rebase upstream/master
4. git push origin master --force

But, DO NOT follow these steps UNTIL you've reverted your previous accidental steps.

Does that make sense?

[kamrandotpk]

Thanks @Rican7 , my bad - I'm so stupid. Before I mess up again, do you have specific command(s) to revert the right way? I don't wanna end up running weird commands again. Thanks.

[Rican7]

Yea, no problem @thesocialgeek.
What you're going to want to do is a hard reset back to your last commit (before merging):

1. git checkout master
2. git reset --hard 07b61e0

Then, follow my previously mentioned steps.

[kamrandotpk]

Thanks @Rican7 :) I will follow these steps as I reach home from work. Thanks.

[Rican7]

Thank you @thesocialgeek!

[kamrandotpk]

The build process is still broken for some reason, any ideas guys? Thanks!

[Rican7]

@thesocialgeek You did a great job fixing the git issue! Nice job!

As far as the build error goes, don't worry, as the only error that's being thrown in the build process is one that we're already aware of: #298

In other words, as far as I can tell, this one's good to merge.

[kamrandotpk]

Thanks @Rican7 :) Thanks for letting me know :)

[koenpunt]

But what if the column in the database uses uppercase characters? Doesn't this create problems? Maybe add some tests for this specific case-change.

[Rican7]

@koenpunt good point

[kamrandotpk]

@koenpunt very good point. Thanks. I am sorry for the late reply guys. I will look into the matter at my earliest and see what possible options are there for this if any. Thanks.

[kamrandotpk]

@koenpunt and @Rican7
Guys, I looked into the source and it turns out that it doesn't matter if the column in the database uses uppercase characters since PHPActiveRecord uses the 'inflected_name' of the columns for storage in the $attributes. and Inflected names are always lowercased no matter what case the column in the database has.

Atleast that is how PHPActiveRecord's StandardInflector works: https://github.com/kla/php-activerecord/blob/master/lib/Inflector.php#L118

The problem will be: what if someone decides to use a custom inflector by adding his or her own implementation of Inflector, and decides NOT to lowercase the column name, we could have a problem. The solution to that is, in the change that I committed, instead of simply lowercasing the column name, we use the Inflector class's variablize() method itself like so:

$this->pk = array_map(function($value) {
    return Inflector::instance()->variablize($value);
}, $this->pk);

Here we are applying the variablize method making sure we use the right inflected name.

The message is a rather long one, so please let me know if anything is not clear.

What do you guys think?

[al-the-x]

I agree on using variablize($value) instead of just strtolower() to maintain normalcy.

[kamrandotpk]

I have committed the new change guys. Please let me know if I need to do anything further. Thanks for guiding me so far.

[kamrandotpk]

@jpfuentes2 do I need to take additional steps for this request to process further? Sorry I am kinda new to contributing to open source so I am not aware of all the processed yet.

[al-the-x]

Nothing you need to do, @thesocialgeek. We just gotta get our $#!+ together over here and merge some of these open PRs.

[kamrandotpk]

@al-the-x Thanks for that David! Sounds great! :)

[koenpunt]

Merged d51117c as d0fee44 into 1.1-dev. When using variablize primary keys using dashes (-) would give issues.