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

Avatar upload #380

Merged
merged 3 commits into from
May 30, 2016
Merged

Avatar upload #380

merged 3 commits into from
May 30, 2016

Conversation

Henni
Copy link
Contributor

@Henni Henni commented Apr 20, 2016

No description provided.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @DeepDiver1975, @skjnldsv and @jancborchardt to be potential reviewers

@codecov-io
Copy link

codecov-io commented Apr 20, 2016

Current coverage is 12.45%

Merging #380 into master will decrease coverage by -<.01%

@@             master       #380   diff @@
==========================================
  Files            43         45     +2   
  Lines           773        795    +22   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits             97         99     +2   
- Misses          676        696    +20   
  Partials          0          0          

Powered by Codecov. Last updated by ae2fe17...ccec052

@jancborchardt
Copy link
Member

Shouldn't this go on top of @skjnldsv's #342 ?

@skjnldsv
Copy link
Contributor

Yep, why @Henni :'(

@Henni
Copy link
Contributor Author

Henni commented Apr 21, 2016

Most of the code of #342 is already on master and the rebase was a bit messy.

@skjnldsv
Copy link
Contributor

:)
Will review tonight.

@Henni
Copy link
Contributor Author

Henni commented Apr 21, 2016

Set state to developing because it is currently not possible to change the image.

@DeepDiver1975 DeepDiver1975 added this to the 2.0-next milestone Apr 21, 2016
@Henni
Copy link
Contributor Author

Henni commented Apr 28, 2016

It is now possible to change the avatar of a contact.

Remaining todos:

  • check file size
  • delete avatar functionality

We can fix these remaining todos in a separate PR, in my opinion.

@skjnldsv
Copy link
Contributor

I have an issue! :)
When uploading a big picture, I get a 500 error that corrupts my owncloud.log because it outputs the entire base64 image in it! It also put my php-fpm process in crazy mode and uses 100%cpu until I have to kill it! :/

@skjnldsv
Copy link
Contributor

{
    "reqId":"vhnbX/ZZHj8SBHq36/zn",
    "remoteAddr":"127.0.0.1",
    "app":"webdav",
    "message":"Exception: {
        "Message":"An exception occurred while executing 
            'UPDATE `oc_cards`
            SET `carddata` = ?,
                `lastmodified` = ?,
                `size` = ?,
                `etag` = ?
            WHERE (`uri` = ?)
                AND (`addressbookid` = ?)'
            with params [
                "BEGIN:VCARD
                    VERSION:3.0
                    FN:Test Testatum Testatas
                    CATEGORIES:Test
                    UID:630fdc3f-d662-4137-b86e-56769b16bbde
                    TEL;TYPE=HOME\\,VOICE:+0000000000000000
                    ADR;TYPE=HOME:;;27 Test street;Testown;Testos;1337;Testria
                    EMAIL;TYPE=HOME:[email protected]
                    NOTE:Test test test test test\, testteeeesttest!
                    TITLE:CEO
                    ORG:Test & Sons inc
                    PHOTO;TYPE=image/jpeg:/9j/4AAQSkZJRgABAQAAAQABAAD/2wBDAAEBAQEBAQEBAQEBAQEBA
                     QICAQEBAQMCAgICAwMEBAMDAwMEBAYFBAQFBAMDBQcFBQYGBgYGBAUHBwcGBwYGBgb/2wBDAQE
                     BAQEBAQMCAgMGBAMEBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGBgYGB
                     gYGBgYGBgb/wAARCAhwDwADASIAAhEBAxEB/8QAHwAAAgIDAQEBAQEAAAAAAAAABQYEBwIDCAk
                     BAAoL/8QAXQ--------INSANEBASE64-STRING-REMOVED-HERE---------6GHy9htrOmYAFQ
                     Yeu39/wCtvkglOevpK2gTiQKADUfe2ZBCUvTe3LlnIdB9drb2yZFHCaSpsNM061KZy0mGgb20W
                     /CUJAJtCm725cpz9ui3XOi0cZlIJSWwhJPQB2H3EbQgam7jL2lb5WKIgBQmA762+AAH0DWduXL
                     6JBINI6iFvpTAQQETSENtLfgEtNMpnDc07ZFBQBGo+/lCkLcuWtVUFQmAyAO1tJTmIMyb2l1f8
                     f8A8rbSsbp8/wD8rbly2AcFhKBNN5+9tCiQkEQEJh62xAAkYfSUrZFMISkeXtTbly//2Q==
                    END:VCARD",
                1462000582,
                5400962,
                "50c7f8c4697ffc29d29c60a99c21bde5",
                "630fdc3f-d662-4137-b86e-56769b16bbde.vcf",
                "1"
            ]
            Communication link failure: 1153 Got a packet bigger than 'max_allowed_packet' bytes",
        "Exception":"Doctrine\DBAL\Exception\DriverException",
        "Code":0,
        "Trace":"
            #0 /usr/share/webapps/owncloud/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php(116): Doctrine\DBAL\Driver\AbstractMySQLDriver->convertException('An exception oc...', Object(Doctrine\DBAL\Driver\PDOException))\n
            #1 /usr/share/webapps/owncloud/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Connection.php(996): Doctrine\DBAL\DBALException::driverExceptionDuringQuery(Object(Doctrine\DBAL\Driver\PDOMySql\Driver), Object(Doctrine\DBAL\Driver\PDOException), 'UPDATE `oc_card...', Array)\n
            #2 /usr/share/webapps/owncloud/lib/private/db/connection.php(205): Doctrine\DBAL\Connection->executeUpdate('UPDATE `oc_card...', Array, Array)\n
            #3 /usr/share/webapps/owncloud/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Query/QueryBuilder.php(208): OC\DB\Connection->executeUpdate('UPDATE `oc_card...', Array, Array)\n
            #4 /usr/share/webapps/owncloud/lib/private/db/querybuilder/querybuilder.php(141): Doctrine\DBAL\Query\QueryBuilder->execute()\n
            #5 /usr/share/webapps/owncloud/apps/dav/lib/carddav/carddavbackend.php(548): OC\DB\QueryBuilder\QueryBuilder->execute()\n
            #6 /usr/share/webapps/owncloud/3rdparty/sabre/dav/lib/CardDAV/Card.php(94): OCA\DAV\CardDAV\CardDavBackend->updateCard('1', '630fdc3f-d662-4...', 'BEGIN:VCARD\r\nVE...')\n
            #7 /usr/share/webapps/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php(1070): Sabre\CardDAV\Card->put('BEGIN:VCARD\r\nVE...')\n
            #8 /usr/share/webapps/owncloud/3rdparty/sabre/dav/lib/DAV/CorePlugin.php(511): Sabre\DAV\Server->updateFile('addressbooks/us...', 'BEGIN:VCARD\r\nVE...', NULL)\n
            #9 [internal function]: Sabre\DAV\CorePlugin->httpPut(Object(Sabre\HTTP\Request), Object(Sabre\HTTP\Response))\n
            #10 /usr/share/webapps/owncloud/3rdparty/sabre/event/lib/EventEmitterTrait.php(105): call_user_func_array(Array, Array)\n
            #11 /usr/share/webapps/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php(459): Sabre\Event\EventEmitter->emit('method:PUT', Array)\n
            #12 /usr/share/webapps/owncloud/3rdparty/sabre/dav/lib/DAV/Server.php(248): Sabre\DAV\Server->invokeMethod(Object(Sabre\HTTP\Request), Object(Sabre\HTTP\Response))\n
            #13 /usr/share/webapps/owncloud/apps/dav/lib/server.php(142): Sabre\DAV\Server->exec()\n
            #14 /usr/share/webapps/owncloud/apps/dav/appinfo/v2/remote.php(29): OCA\DAV\Server->exec()\n
            #15 /usr/share/webapps/owncloud/remote.php(138): require_once('/usr/share/weba...')\n
            #16 {main}",
        "File":"/usr/share/webapps/owncloud/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php",
        "Line":115,
        "User":"admin"
    },
    "level":4,
    "time":"2016-04-30T07:16:22+00:00",
    "method":"PUT",
    "url":"/remote.php/dav/addressbooks/users/admin/contacts/630fdc3f-d662-4137-b86e-56769b16bbde.vcf",
    "user":"admin"
}

@skjnldsv
Copy link
Contributor

skjnldsv commented May 9, 2016

  • check file size
  • delete avatar functionality
    We can fix these remaining todos in a separate PR, in my opinion.

I think we should prevent picture too big for now, or we could have people having the same issue than I have. A simple alert should be fine for now! :)

@skjnldsv
Copy link
Contributor

skjnldsv commented May 9, 2016

I added a file size check to prevent further errors.
Tell me what you think @Henni :)

I think now we can merge! 👍

@DeepDiver1975
Copy link
Member

@skjnldsv travis is failing - please have a look ...

@skjnldsv
Copy link
Contributor

skjnldsv commented May 9, 2016

Oups, my bad @DeepDiver1975 , wrong copy/paste.
Should be good.

@@ -11,17 +11,21 @@ angular.module('contactsApp')
var input = element.find('input');
input.bind('change', function() {
var file = input.get(0).files[0];
var reader = new FileReader();
if (file.size > 1024*1024) { // 1 MB
OC.Notification.showTemporary(t('contacts', 'The selected image is too big'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should tell the user the max size

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good Idea!

@DeepDiver1975 DeepDiver1975 modified the milestones: 1.3, 2.0-next May 9, 2016
@skjnldsv
Copy link
Contributor

skjnldsv commented May 9, 2016

@DeepDiver1975 done

@DeepDiver1975 DeepDiver1975 merged commit fc1f7c7 into master May 30, 2016
@DeepDiver1975 DeepDiver1975 deleted the avatar-upload branch May 30, 2016 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants