New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.7] Bug fix for Blueprint method removeColumn #27115

Merged
merged 1 commit into from Jan 10, 2019

Conversation

Projects
None yet
3 participants
@b-stavitskiy
Copy link

b-stavitskiy commented Jan 9, 2019

Variable $c is instance of \Illuminate\Support\Fluent class, so when we access it as array - returns content of attributes, not attributes itself.

This bug makes impossible to delete column from \Illuminate\Database\Schema\Blueprint.

I can't find tests for this functionality. (1)
And how can it be fixed in prev versions? (2)
should i make pull for another branches like 5.6, because i got projects on this versions and it would make my life easier if this method was workable?

@ahinkle

This comment has been minimized.

Copy link
Contributor

ahinkle commented Jan 10, 2019

Just curious, do you have an example when would you use removeColumn()? Seems kinda pointless removing a column within your blueprint when you can just remove it.

I can't find tests for this functionality. (1)

Would you be able to add a test to back this PR?

should i make pull for another branches like 5.6, because i got projects on this versions and it would make my life easier if this method was workable?

No, Laravel only supports 5.7 and security/bug fixes for 5.5 LTS. If this an actual bug, you would have to submit a separate PR for 5.5.

@ahinkle

This comment has been minimized.

Copy link
Contributor

ahinkle commented Jan 10, 2019

Hey, I was able to replicate this bug..

Schema::create('users', function (Blueprint $table) {
            $table->increments('id');
            $table->string('name');
            $table->string('email')->unique();
            $table->timestamp('email_verified_at')->nullable();
            $table->string('password');
            $table->rememberToken();
            $table->timestamps();

            $table->string('foo');
            $table->removeColumn('foo'); // Shouldn't appear in Database.
        });

Your PR fixes it -- just add a test to prevent future issues. 😉

@taylorotwell taylorotwell merged commit 348c447 into laravel:5.7 Jan 10, 2019

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment