User bio #286

eduardostuart · 2017-07-08T21:12:59Z

Allow users to add a short bio of 140 characters. (reference: #283)

Added User bio content into user profile page.
New migration file - bio column (144 characters, nullable)
Styles updated - add margins and max-width to bio text
Feature tests updated
UpdateProfileRequest updated. Added sometimes|max:144 for bio field.
Add boot event handlers to remove html tags from bio

Preview

driesvints · 2017-07-08T21:43:24Z

@eduardostuart thank you for sending this in! I was already reviewing but saw you pushed some more commits. Request a review from me when you're done.

eduardostuart · 2017-07-08T21:53:32Z

@driesvints ok!

Should I also add bio field into registration? What do you think?

eduardostuart · 2017-07-08T22:55:47Z

@driesvints can you review?

driesvints

First of all: thank you for sending this in and helping out! The screenshots really helped, good job :)

I have some changes I'd like you to make. First, it might be best to increase the bio length to 160 characters to match Twitter's max bio length. Second, it's best if we just always have bio be a string, that way you don't need to deal with null values.

Also, something went wrong with the yarn.lock file, can you please revert those changes?

There is more feedback, feel free to take your time to go through it and get back to me when you made the changes 🙂

driesvints · 2017-07-09T10:44:42Z

app/Http/Requests/UpdateProfileRequest.php

@@ -12,9 +12,15 @@ public function rules()
            'name' => 'required|max:255',
            'email' => 'required|email|max:255|unique:users,email,'.Auth::id(),
            'username' => 'required|max:255|unique:users,username,'.Auth::id(),
+            'bio' => 'sometimes|max:144',


Increase this to 160. I also don't think we need to use sometimes here?

driesvints · 2017-07-09T10:44:50Z

app/Http/Requests/UpdateProfileRequest.php

        ];
    }

+    public function bio(): ?string


Remove the nullable return type

driesvints · 2017-07-09T10:45:03Z

app/Http/Requests/UpdateProfileRequest.php

        ];
    }

+    public function bio(): ?string
+    {
+        return $this->get('bio');


Return a default value of an empty string here

driesvints · 2017-07-09T10:45:38Z

app/Jobs/UpdateProfile.php

@@ -29,6 +29,7 @@ public static function fromRequest(User $user, UpdateProfileRequest $request): s
            'name' => $request->name(),
            'email' => $request->email(),
            'username' => strtolower($request->username()),
+            'bio' => $request->bio(),


Replace this with 'bio' => trim(strip_tags($request->bio()),

driesvints · 2017-07-09T10:45:51Z

app/User.php

+        static::creating($eventHandler);
+        static::updating($eventHandler);
+    }
+


Remove this, we can do this in the job.

driesvints · 2017-07-09T10:53:01Z

tests/CreatesUsers.php

@@ -38,6 +38,7 @@ protected function createUser(array $attributes = []): User
            'email' => 'john@example.com',
            'password' => bcrypt('password'),
            'github_username' => 'johndoe',
+            'bio' => 'John Doe bio content',


This isn't needed here, remove it.

driesvints · 2017-07-09T10:53:28Z

tests/Feature/SettingsTest.php

            ])
            ->seePageIs('/settings')
            ->see('Something went wrong. Please review the fields below.')
            ->see('The email has already been taken.')
-            ->see('The username has already been taken.');
+            ->see('The username has already been taken.')
+            ->see('My short bio text');


Remove this from this test.

driesvints · 2017-07-09T10:53:36Z

tests/Feature/SettingsTest.php

@@ -46,11 +48,13 @@ public function users_cannot_choose_duplicate_usernames_or_email_addresses()
                'name' => 'Freek Murze',
                'email' => 'freek@example.com',
                'username' => 'freekmurze',
+                'bio' => 'My short bio text',


Remove this from this test.

driesvints · 2017-07-09T10:54:35Z

yarn.lock

+    os-locale "^1.4.0"
+    window-size "^0.1.2"
+    y18n "^3.2.0"
+


I'm not sure what went wrong here but please revert the changes to the yarn.lock file.

driesvints · 2017-07-09T10:56:51Z

app/User.php

+    public function hasBio(): bool
+    {
+        return ! empty($this->bio());
+    }


Remove this method.

eduardostuart · 2017-07-09T12:20:29Z

@driesvints all done 😄

driesvints

Nice job. Some last feedback.

Btw, one more thing: when I said to remove those things from the SettingsTest feature test I meant only for the users_cannot_choose_duplicate_usernames_or_email_addresses method. You can add the changes of the users_can_update_their_profile back.

driesvints · 2017-07-09T12:43:21Z

database/migrations/2017_07_08_202506_add_user_bio_column.php

+     */
+    public function down()
+    {
+    }


You can remove the down method entirely :)

driesvints · 2017-07-09T12:44:15Z

resources/views/users/_user_info.blade.php

@@ -10,6 +10,12 @@
    @endcan

    <h2>{{ $user->name() }}</h2>
+    @if ($bio = $user->bio())
+


Remove this white line.

driesvints · 2017-07-09T12:44:24Z

resources/views/users/_user_info.blade.php

@@ -10,6 +10,12 @@
    @endcan

    <h2>{{ $user->name() }}</h2>
+    @if ($bio = $user->bio())


Add a white line above the if statement.

driesvints · 2017-07-09T12:49:40Z

tests/Components/Jobs/UpdateProfileTest.php

+
+        $this->assertEquals('my bio', $user->bio());
+        $this->assertEquals('John Doe Junior', $user->name());
+    }


It's not needed to login the user. Since you're testing a component no "state" of the active user is needed. You can re-write this as follow:

/** @test */ public function we_can_update_a_user_profile() { $user = (new UpdateProfile($this->createUser(), ['bio' => 'my bio', 'name' => 'John Doe Junior']))->handle(); $this->assertEquals('my bio', $user->bio()); $this->assertEquals('John Doe Junior', $user->name()); }

Make sure that the handle method returns the updated user.

eduardostuart · 2017-07-09T17:16:33Z

@driesvints Done! 👍 Can you check?

driesvints · 2017-07-09T18:50:33Z

Thank you for your work on this! :)

eduardostuart added 3 commits July 8, 2017 22:14

fix vagrant path

b5e39b0

allow users to add short bio of 140 characters

6ed305a

update feature test

b11f189

eduardostuart changed the title ~~User bio - #283~~ User bio Jul 8, 2017

eduardostuart added 3 commits July 8, 2017 23:24

fix user register class

d7cda4a

fix tests and styleci

f16c7a5

style ci fix

1b00f2a

remove bio from registration, small fixes

757e030

driesvints requested changes Jul 9, 2017

View reviewed changes

eduardostuart added 4 commits July 9, 2017 14:02

code review fixes

15f88a6

revert yarn.lock file

aa7a626

fix styleci

cd66de7

fix styleci

001e468

driesvints requested changes Jul 9, 2017

View reviewed changes

eduardostuart added 3 commits July 9, 2017 19:00

code review fixes

6a89165

check bio field in users_can_update_their_profile test

1e034be

styleci fix

98423f2

driesvints approved these changes Jul 9, 2017

View reviewed changes

driesvints merged commit a911275 into laravelio:master Jul 9, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User bio #286

User bio #286

eduardostuart commented Jul 8, 2017 •

edited

Loading

driesvints commented Jul 8, 2017

eduardostuart commented Jul 8, 2017

eduardostuart commented Jul 8, 2017

driesvints left a comment

driesvints Jul 9, 2017

driesvints Jul 9, 2017

driesvints Jul 9, 2017

driesvints Jul 9, 2017

driesvints Jul 9, 2017

driesvints Jul 9, 2017

driesvints Jul 9, 2017

driesvints Jul 9, 2017

driesvints Jul 9, 2017

driesvints Jul 9, 2017

eduardostuart commented Jul 9, 2017

driesvints left a comment

driesvints Jul 9, 2017

driesvints Jul 9, 2017

driesvints Jul 9, 2017

driesvints Jul 9, 2017

eduardostuart commented Jul 9, 2017

driesvints commented Jul 9, 2017

User bio #286

User bio #286

Conversation

eduardostuart commented Jul 8, 2017 • edited Loading

Preview

driesvints commented Jul 8, 2017

eduardostuart commented Jul 8, 2017

eduardostuart commented Jul 8, 2017

driesvints left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eduardostuart commented Jul 9, 2017

driesvints left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eduardostuart commented Jul 9, 2017

driesvints commented Jul 9, 2017

eduardostuart commented Jul 8, 2017 •

edited

Loading