Profile Picture added on API and Backend #6

aksinha-nerdapplabs · 2017-02-15T13:50:35Z

praveen-garg

minor comments;

praveen-garg · 2017-02-15T14:05:15Z

src/ApiBundle/Controller/Admin/UserController.php

              $this->addFlash('success', $flashMsg);

+            } catch(UploadException $e) {
+              $this->logMessage(400, 'danger', $e->getMessage());


should not we use logMessageAndFlash?

praveen-garg · 2017-02-15T14:06:15Z

src/ApiBundle/Controller/Admin/UserController.php

            // Always catch exact exception for which flash message or logger is needed,
            // otherwise catch block will not get executed on higher or lower ranked exceptions.
+            } catch(UploadException $e) {
+              $this->logMessage(400, 'danger', $e->getMessage());


logMessageAndFlash?

praveen-garg · 2017-02-15T14:07:13Z

src/ApiBundle/Controller/AuthController.php

      *      {"name"="dob", "dataType"="datetime", "required"=true, "description"="date of birth mm/dd/yyyy"},
      *      {"name"="email", "dataType"="email", "required"=true, "description"="Email"},
      *      {"name"="email_confirmation", "dataType"="integer", "required"=true, "description"="0-email confirmation not required, 1-required"},
+      *      {"name"="image", "dataType"="image/jpeg, image/jpg, image/gif, image/png", "required"=false, "description"="Profile Picture within 1024k size"},


where did we limit 1024K

praveen-garg · 2017-02-15T14:08:07Z

src/ApiBundle/Entity/User.php


+    /**
+    * @var string $image
+    * @Assert\File(


Okay, here it is! cool!

As discussed, User (who is signing up) should be able add/ remove or update user profile image; not the admin only.

praveen-garg · 2017-02-17T14:32:05Z

src/ApiBundle/Form/UserType.php

            ->add('dob', DateType::class, array('widget' => 'single_text', 'format' => 'M/d/y'))
            ->add('username', TextType::class)
-            ->add('password', PasswordType::class, array('data' => ''))
+            ->add('password', TextType::class)


changed it back to TextType?
it was chnaged to PasswordType in last PR
https://github.com/nerdapplabs/authOauth/pull/9/files#diff-3683af68b1a14d46bf30b457a8ec4ddfR31

aksinha-nerdapplabs · 2017-02-18T10:25:27Z

@praveen-garg have a look. Changes are final till now.

praveen-garg · 2017-02-18T12:54:23Z

src/ApiBundle/Controller/AuthController.php

        $user->setEmail($request->request->get('email'));
        $user->setFirstname($request->request->get('firstname'));
-        $user->setLastname($request->request->get('lastname'));
+        $user->setLastname($request->request->get('lastname') ? $request->request->get('lastname') : null);


should not we make it EMPTY string ? ""

$user->setLastname($request->request->get('lastname') ? $request->request->get('lastname') : "");

praveen-garg · 2017-02-18T12:56:06Z

src/ApiBundle/Controller/AuthController.php

        $oAuthRtn = 'Pending';
        $msg = 'N.A.';
        $grantType = 'password';
+        $loginConfirmation = $this->container->getParameter('login_confirmation');


what is the use of this var?

praveen-garg · 2017-02-18T12:56:38Z

src/ApiBundle/Controller/AuthController.php

+        $loginConfirmation = $this->container->getParameter('login_confirmation');

-        if ('1' == $request->request->get('email_confirmation')) {
+        if (true == $this->container->getParameter('email_confirmation') ) {


email_confirmation > registration_requires_email_confirmation

praveen-garg · 2017-02-18T12:57:50Z

src/ApiBundle/Controller/AuthController.php


+        // If no image file, then return empty response
+        if (!$user->getImage()) {
+            return new Response();


can you please explain more?

If no image file, then return empty response ?

yeah, Empty with 200 ok.

praveen-garg · 2017-02-18T12:59:35Z

src/ApiBundle/Controller/AuthController.php

      *  description="Fetch User profile detail. Access token to be provided in header (Authorization = Bearer <access token>)",
      *  parameters={
-      *      {"name"="image", "dataType"="image/jpeg, image/jpg, image/gif, image/png", "required"=false, "description"="Profile Picture within 1024k size"},
+      *      {"name"="image", "dataType"="image/jpeg, image/jpg, image/gif, image/png", "required"=false, "description"="jpeg/jpg/png/gif Pic Size: 1024k, Width: 50-300px, Height: 50-300px"},


older looks better!

praveen-garg · 2017-02-18T13:00:53Z

src/ApiBundle/Resources/config/config.yml

-    images_profile_directory: '%kernel.root_dir%/../web%images_profile_suffix%'
+    images_profile_directory: '%kernel.root_dir%/../web/images/profile'
+    email_confirmation: false
+    login_confirmation: false


see previous comments

praveen-garg · 2017-02-18T15:01:57Z

@aksinha-nerdapplabs we must keep PR specific to the issue; not sure why this PR has registration_requires_email_confirmation related change. a new PR (with minimum & to the point chnages) could have been easy to review/ test and merge

praveen-garg · 2017-02-20T10:14:33Z

src/ApiBundle/Controller/AuthController.php

-        if ('1' == $request->request->get('email_confirmation')) {
+        // If Login confirmation is set, then upon successful registration, user will be logged in automatically,
+        // else user will be redirected to a login page to log in with the new credentials.
+        $loginConfirmation = $this->container->getParameter('login_confirmation');


login_confirmation > login_required_after_signup

praveen-garg · 2017-02-21T06:23:32Z

src/ApiBundle/Controller/UserController.php

+                $user,
+                $response);
+        } catch (AccountStatusException $ex) {
+            // We simply do not authenticate users which do not pass the user


but we should log?

praveen-garg · 2017-02-21T09:44:21Z

src/ApiBundle/Controller/Admin/UserController.php

+                          'entry_options'  => array(
+                              'label' => false,
+                              'choices'  => array(
+                                  'ROLE_ADMIN' => 'ROLE_ADMIN',


praveen-garg · 2017-02-21T09:44:34Z

src/ApiBundle/Controller/Admin/UserController.php

+                          'entry_options'  => array(
+                              'label' => false,
+                              'choices'  => array(
+                                  'ROLE_ADMIN' => 'ROLE_ADMIN',


are your sure?

praveen-garg

DRY review; need more testing..

Profile Picture added on API and Backend

9e0fdae

aksinha-nerdapplabs added the enhancement label Feb 15, 2017

aksinha-nerdapplabs requested a review from praveen-garg February 15, 2017 13:50

praveen-garg requested changes Feb 15, 2017

View reviewed changes

aksinha-nerdapplabs added 2 commits February 17, 2017 17:40

Conflict resolved

fa82704

First modifications post validtions PR merger

ad81fab

praveen-garg reviewed Feb 17, 2017

View reviewed changes

Profile Pic changes. email_confirmation removed. api doc updated.

20cf74d

praveen-garg reviewed Feb 18, 2017

View reviewed changes

Changes per feedback

df12eaf

praveen-garg reviewed Feb 20, 2017

View reviewed changes

aksinha-nerdapplabs added 3 commits February 20, 2017 16:03

Merge Conflict resolved

afbc9e4

Resolved merge conflict

2b455f8

New UserController added with views

5bfdbbe

praveen-garg reviewed Feb 21, 2017

View reviewed changes

Profile pic further updations

954ada4

praveen-garg reviewed Feb 21, 2017

View reviewed changes

aksinha-nerdapplabs added 5 commits February 21, 2017 16:04

Login screen modified

11e30dc

Password policy changed

d897711

AuthController doc updated for new password policy

4606c6c

Minor fixes

bd79482

Changes per feedback. DatePicker added.

7228e0b

aksinha-nerdapplabs added 9 commits February 21, 2017 18:56

Changes per feedback.

db94ad9

Image validation per feedback

04799ca

Profile pic validation before saving added

523a677

Few language msgs corrected

451f2a3

Changes per feedback on profile pic edit

7532ad0

Resolved merge conflict

fdcfe51

Admin self deletion check added

985c886

Minor fix

8903504

Minor fix

13a0324

praveen-garg approved these changes Feb 22, 2017

View reviewed changes

praveen-garg merged commit 250ad03 into nerdapplabs:master Feb 22, 2017

aksinha-nerdapplabs deleted the profile_image_added branch February 23, 2017 07:03

Profile Picture added on API and Backend #6

Profile Picture added on API and Backend #6

Uh oh!

Conversation

aksinha-nerdapplabs commented Feb 15, 2017

Uh oh!

praveen-garg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aksinha-nerdapplabs commented Feb 18, 2017

Uh oh!

praveen-garg Feb 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praveen-garg commented Feb 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praveen-garg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

praveen-garg Feb 18, 2017 •

edited

Loading