Skip to content
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

Update to Font Awesome 5 (Develop) #957

Merged
merged 21 commits into from Jul 17, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

Draft of UpdateGroupsTable.php migration

  • Loading branch information...
amosfolz committed Apr 7, 2019
commit afbab2098c2536d3ca2a3ec86837fbed165f8138
@@ -0,0 +1,53 @@
<?php
/**
* UserFrosting (http://www.userfrosting.com)
*
* @link https://github.com/userfrosting/UserFrosting
* @copyright Copyright (c) 2019 Alexander Weissman
* @license https://github.com/userfrosting/UserFrosting/blob/master/LICENSE.md (MIT License)
*/
namespace UserFrosting\Sprinkle\Account\Database\Migrations\v430;
use Illuminate\Database\Schema\Blueprint;
use UserFrosting\Sprinkle\Core\Database\Migration;
/**
* Groups table migration
* Changes the `icon` column property of `default` to NULL to align with new Font Awesome 5 tag convention.
* Version 4.3.0
*
* See https://laravel.com/docs/5.4/migrations#tables
* @author Alex Weissman (https://alexanderweissman.com)
*/
class UpdateGroupsTable extends Migration
{
/**
* {@inheritdoc}
*/
public static $dependencies = [
'\UserFrosting\Sprinkle\Account\Database\Migrations\v400\GroupsTable'
];
/**
* {@inheritdoc}
*/
public function up()
{
if ($this->schema->hasTable('groups')) {
$this->schema->table('groups', function (Blueprint $table) {
$table->string('icon', 100)->default('NULL')->change();

This comment has been minimized.

Copy link
@lcharette

lcharette Apr 7, 2019

Member

Should it be null, or an empty string? 🤔

This comment has been minimized.

Copy link
@amosfolz

amosfolz Apr 7, 2019

Author Contributor

I went with NULL based off this post: https://stackoverflow.com/questions/38351498/remove-default-in-migration

I also compared to the other columns in the table that did not have a default set when the migration ran, and they all showed NULL for the Default property. In mysql here is how the results of my testing:
image

This comment has been minimized.

Copy link
@Silic0nS0ldier

Silic0nS0ldier Apr 8, 2019

Member

Should be possible to do an export of the create table logic to see if the default value was truly reversed (the cleaner the better, leftovers attract edge cases).

Assuming I remember, I'll take a look at what the DDL ends up being.

This comment has been minimized.

Copy link
@amosfolz

amosfolz Apr 22, 2019

Author Contributor

Here is how "default" migration from \UserFrosting\Sprinkle\Account\Database\Migrations\v400\GroupsTable.php looks
userfrosting groups default

Here is the result of proposed UpgradeGroupsTable.php migration up
UpdatedUP

Here is the result of proposed UpgradeGroupsTable.php migration down
updatedDOWN

This comment has been minimized.

Copy link
@Silic0nS0ldier

Silic0nS0ldier Apr 23, 2019

Member

I think the posted DDL is wrong. Or its mislabeled? I'll still have to look at this myself, but it works as a good reminder none-the-less, so thanks.

This comment has been minimized.

Copy link
@amosfolz

amosfolz Apr 24, 2019

Author Contributor

I am curious about what is wrong?

Other than ('fas fa-user') in this line, the down method returns the table to the 'original' state. I was not certain how to handle this but I thought it would be better to set the default to use the new fas convention rather than the fa.

I relabeled my pictures above that might help clear up any confusion.

});
}
}
/**
* {@inheritdoc}
*/
public function down()
{
$this->schema->table('groups', function (Blueprint $table) {
$table->string('icon', 100)->default('fas fa-user')->change();
});
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.