Add image uploads #76

angeluss88 · 2017-10-18T09:18:43Z

No description provided.

pilot

Please add tests with behat
add screenshots, or video set how does it work

pilot · 2017-10-18T13:21:38Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+
+    public function getUploadPath()
+    {
+        return $this->get('kernel')->getRootDir() . DIRECTORY_SEPARATOR . '..' . DIRECTORY_SEPARATOR .


should be taken from the paramters.yml

pilot · 2017-10-18T13:21:56Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+    }
+
+    public function getUploadUrl(){
+        return '/uploads/media/';


should be taken from the paramters.yml

pilot · 2017-10-18T13:23:24Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+    {
+        $oldFileName = '';
+        if ($id === null) {
+            $entity = new Media();


$entity is not yet defined, as well use user-friendly naming, not entity but $media

pilot · 2017-10-18T13:24:37Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+            $form->handleRequest($request);
+
+            if ($form->isValid()) {
+                if(is_null($entity->getId())){


check CS (code style) for all files what you added

if ( should be with the space, use code sniffer from sensiolab

Setup Symfony code sniffer https://confluence.jetbrains.com/display/PhpStorm/PHP+Code+Sniffer+with+Symfony2+Coding+Standards+Integration+in+PhpStorm+-+Symfony+Development+using+PhpStorm

I think this condition in general is excess here

pilot · 2017-10-18T13:25:08Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+                if($oldFileName !== $entity->getFilename()) {
+                    $url = $request->request->get('file_url');
+                    $path = $this->getUploadPath();
+                    $entity->setFilename( time() . '_' . $entity->getFilename());


setFilename(time() no space, CS

pilot · 2017-10-18T13:32:04Z

src/Event/EventBundle/Entity/Media.php

+     * @param \DateTime $createdDate
+     * @return Media
+     */
+    public function setCreatedDate($createdDate)


add typehint

I would replace setCreated and setUpdated with Gedmo Timestampable

pilot · 2017-10-18T13:33:08Z

src/Event/EventBundle/Resources/views/Backend/Media/index.html.twig

+
+            if ( 0 < elementsCount) {
+                $('#media-table').dataTable({
+                    "iDisplayLength": 50,


use single quotes

pilot · 2017-10-18T13:34:59Z

src/Event/EventBundle/Resources/views/Backend/Media/manage.html.twig

+{% block javascripts %}
+    {{ parent() }}
+
+    <script type="text/javascript" src="https://static.filestackapi.com/v3/filestack.js"></script>


to use filestack we should be specify that in the Readmy

pilot · 2017-10-18T13:35:04Z

src/Event/EventBundle/Resources/views/Backend/Media/manage.html.twig

+                accept: 'image/*',
+            }).then(function(result) {
+                var file = result.filesUploaded[0];
+//                var file = JSON.stringify(result.filesUploaded);


pilot · 2017-10-18T13:35:49Z

src/Event/EventBundle/Resources/views/Backend/Media/manage.html.twig

+
+    <script type="text/javascript" src="https://static.filestackapi.com/v3/filestack.js"></script>
+    <script>
+        var client = filestack.init('A3w1aZwZmQBibC09rAcZnz');


should be taken from parameters.yml at least for now, or better add event settings page where this is can be specified

barik94 · 2017-10-18T14:06:23Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+
+            if ($form->isValid()) {
+                if(is_null($entity->getId())){
+                    $entity->setCreatedDate(new \DateTime());


set it in entity constructor

barik94 · 2017-10-18T14:07:32Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+            $form->handleRequest($request);
+
+            if ($form->isValid()) {
+                if(is_null($entity->getId())){


I think this condition in general is excess here

barik94 · 2017-10-18T14:09:35Z

src/Event/EventBundle/Entity/Media.php

+        return '/uploads/media/' . $this->filename;
+    }
+
+    public function __construct()


@pilot why?
here should be

$this->created = new \DateTime(); $this->updated = new \DateTime();

barik94 · 2017-10-18T14:11:40Z

src/Event/EventBundle/Resources/views/Backend/Media/manage.html.twig

+                $('#media_filename').val(file.filename);
+                $('#file_url').val(file.url);
+
+                console.log(file.filename);


rm all console.log too

lyovkin · 2017-10-18T14:36:06Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+        curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
+        $file = curl_exec($ch);
+        curl_close($ch);
+        return $file;


maybe better to use Guzzle or similar http client for requests

lyovkin · 2017-10-18T14:46:04Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+                    $entity->setFilename( time() . '_' . $entity->getFilename());
+                    $file = $this->getFile($url);
+                    file_put_contents($path . $entity->getFilename(), $file);
+                    chmod($path . $entity->getFilename(), 0777);


use for this Symfony Filesystem component

lyovkin · 2017-10-18T15:02:55Z

src/Event/EventBundle/Entity/Media.php

+     * @param \DateTime $createdDate
+     * @return Media
+     */
+    public function setCreatedDate($createdDate)


I would replace setCreated and setUpdated with Gedmo Timestampable

NikitaKharkov · 2017-10-18T14:00:16Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+        ]);
+    }
+
+    protected function getFile($url){


Should be on a new line

NikitaKharkov · 2017-10-18T14:02:58Z

src/Event/EventBundle/Resources/config/routing/backend.yml

+backend_media_delete:
+    path:  /media/delete/{id}
+    defaults: { _controller: EventEventBundle:Backend/Media:delete }
+    requirements: { id: \d+ }


Have to be one empty line at the end of file

NikitaKharkov · 2017-10-18T14:04:28Z

src/Event/EventBundle/Resources/public/css/style.css

+#media_title {
+    width: 530px;
+}
+/*  backend media uploader */


In all files. In PhpStorm:

athlonUA · 2017-11-01T11:21:10Z

features/backend/media.feature

+    Given following "Event":
+        | ref    | title          | description                | startDate        | endDate          | venue              | email               | host                      |
+        | event  | My event       | My another awesome event!  | 2016-03-01 10:00 | 2016-03-01 18:00 | Burj Khalifa Tower | [email protected] | http://localhost:8000     |
+        | event2 | My other event | My other awesome event!    | 2016-03-01 10:00 | 2016-03-01 18:00 | Burj Khalifa Tower | [email protected] | http://eventator.loc      |


Please check https://github.com/pilot/eventator/blob/master/var/ci/eventator.loc.conf

In this config u can see port 8080, not 80! 😃 But in the event2 host value i see 80 port (!) That's the main reason why your test is fail.

pilot · 2017-11-21T09:28:06Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+            unlink($file);
+        }
+
+        $this->setSuccessFlash('Media deleted.');


should be translatable

pilot · 2017-12-27T16:02:40Z

@angeluss88 replace filestack with https://uppy.io/

pilot · 2017-12-27T16:08:30Z

add to the Wiki manual how to configure the uploader https://github.com/pilot/eventator/wiki/Uploader

pilot · 2018-10-04T18:26:58Z

src/Event/EventBundle/Controller/Backend/MediaController.php

+        ]);
+    }
+
+    protected function checkAccessFolder($path){


angeluss added 4 commits October 17, 2017 16:50

add-media. CRUD and filestack uploader

df90aa0

add back link and styles

d3b3925

remove desc column

c5276ff

change index styles

ab7a33d

pilot reviewed Oct 18, 2017

View reviewed changes

barik94 reviewed Oct 18, 2017

View reviewed changes

lyovkin reviewed Oct 18, 2017

View reviewed changes

CS changes

a549fcd

NikitaKharkov reviewed Oct 19, 2017

View reviewed changes

angeluss added 8 commits October 19, 2017 15:50

fixes

8a2f928

add guzzle client

cacf24d

use filesystem component for upload file

a0b7304

add fs_api parameter

140b42d

add behat feature for media

7b6fb6b

add filestack section to readme

a499660

add params to circle

1d5d286

changes to media feature

9e36ac6

athlonUA reviewed Nov 1, 2017

View reviewed changes

angeluss added 7 commits November 1, 2017 13:44

fix media feature eventhost for circle

826e648

add param to behat.yml.dist

212596e

add ci params

13b55ff

revert redundant changes

f0e0ac9

fix media feature

5a6b01d

add check folder permissions

5cd69f6

change ci config add folder permissions

f0a9bd0

pilot reviewed Nov 21, 2017

View reviewed changes

add translation

c6dbfed

pilot mentioned this pull request Dec 11, 2017

Add uploader for the images #26

Open

2 tasks

pilot reviewed Oct 4, 2018

View reviewed changes

src/Event/EventBundle/Controller/Backend/MediaController.php

]);

}

protected function checkAccessFolder($path){

Copy link

Owner

pilot Oct 4, 2018

Choose a reason for hiding this comment

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

CS fix

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add image uploads #76

Add image uploads #76

angeluss88 commented Oct 18, 2017

pilot left a comment

pilot Oct 18, 2017

pilot Oct 18, 2017

pilot Oct 18, 2017

pilot Oct 18, 2017

vcherednichenko86 Oct 18, 2017

barik94 Oct 18, 2017

pilot Oct 18, 2017

pilot Oct 18, 2017

lyovkin Oct 18, 2017

pilot Oct 18, 2017

pilot Oct 18, 2017

pilot Oct 18, 2017

pilot Oct 18, 2017

barik94 Oct 18, 2017

barik94 Oct 18, 2017

barik94 Oct 18, 2017

barik94 Oct 18, 2017

lyovkin Oct 18, 2017

lyovkin Oct 18, 2017

lyovkin Oct 18, 2017

NikitaKharkov Oct 18, 2017

NikitaKharkov Oct 18, 2017

NikitaKharkov Oct 18, 2017

athlonUA Nov 1, 2017

pilot Nov 21, 2017

pilot commented Dec 27, 2017

pilot commented Dec 27, 2017

pilot Oct 4, 2018

Add image uploads #76

Are you sure you want to change the base?

Add image uploads #76

Conversation

angeluss88 commented Oct 18, 2017

pilot 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

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

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

pilot commented Dec 27, 2017

pilot commented Dec 27, 2017

Choose a reason for hiding this comment