Ability to select image as primary #42

prodex · 2017-06-08T18:30:45Z

issue #15

samdark

While it works, there are issues:

API sub-endpoints are looking wrong.
DB has no FKs for primary_image.
It's not clear which image is primary after clicking the icon in UI. I'd either refresh whole page to show new primary in place.

samdark · 2017-06-08T22:03:42Z

migrations/m170513_053552_project_primary_image.php

+{
+    public function safeUp()
+    {
+        $this->addColumn('{{%project}}', 'primary_image_id', $this->integer()->null()->comment('Primary image'));


Foreign key is missing:

$this->addForeignKey('fk-project-primary_image_id', '{{%project}}', 'primary_image_id', '{{%image}}', 'id', 'SET NULL');

samdark · 2017-06-08T22:04:06Z

migrations/m170513_053552_project_primary_image.php

+
+    public function safeDown()
+    {
+        $this->dropColumn('{{%project}}', 'primary_image_id');


Need to drop FK here if we add it:

$this->dropForeignKey('fk-project-primary_image_id', '{{%project}}');

samdark · 2017-06-08T22:04:49Z

models/Image.php

@@ -270,6 +287,13 @@ public function afterDelete()
        $this->removeFile($this->getBigThumbnailPath());
        $this->removeFile($this->getFullPath());
        $this->removeFile($this->getOriginalPath());
+
+        if ($this->project->primary_image_id == $this->id) {


No need for this code since we can use foreign key SET NULL.

samdark · 2017-06-08T22:05:21Z

models/Project.php

@@ -101,12 +104,13 @@ public function rules()
            [['url', 'source_url'], 'url'],
            [['yii_version'], 'in', 'range' => array_keys(self::versions())],
            [['description', 'tagValues'], 'safe'],
+            ['primary_image_id', 'integer']


I think exists validator should be added as well.

samdark · 2017-06-08T22:06:38Z

models/Project.php

@@ -171,22 +193,25 @@ public function getPlaceholderAbsoluteUrl()
        return Url::to($this->getPlaceholderRelativeUrl(), 'http');
    }

+    /**
+     * @return string
+     */
    public function getPrimaryImageThumbnailRelativeUrl()


It's better to fall back to any image than to default one:

if ($this->primaryImage) { return $this->primaryImage->getThumbnailRelativeUrl(); } if (!empty($this->images)) { return $this->images[0]->getThumbnailRelativeUrl(); } return $this->getPlaceholderRelativeUrl();

samdark · 2017-06-08T22:09:20Z

modules/api1/controllers/ProjectController.php

+     * @return Project
+     * @throws NotFoundHttpException
+     */
+    protected function findProject($projectId, $userId)


It could take User instead of ID as argument.

samdark · 2017-06-08T22:12:37Z

modules/api1/docs.md

@@ -57,6 +57,32 @@ You may pass additional parameters when querying a list:
  returned.
 - `yiiVersion` - version of the framework project built with. Either `1.0` or `1.1`.

+
+### Get primary image <a href="#projects-view-primary-image" id="projects-view-primary-image">#</a>


Primary image ID by itself isn't useful. I'd include this info into GET /project if it's interesting to expose it via API at all...

Since it gives image info, I'd add /image endpoint where deleting an image could be moved as well as getting image info.

I agree. I saw that the project is already return the link to the primary image as "thumbnail". I removed an additional query to get the primary image.

samdark · 2017-06-08T22:17:17Z

modules/api1/docs.md

+- updatedAt: UNIX timestamp indicating when image was updated last time.
+
+
+### Update primary image <a href="#projects-update-primary-image" id="projects-update-primary-image">#</a>


If it primaryImageID would be exposed via /project/1 API then it would make sense to update using POST /project/1.

added fixes

maybe "PUT"?

In order, I'm using the property name "primary_image_id" and Project::SCENARIO_MANAGE. Probably a bad idea to call fields in api as in a DB, but how flexible)

samdark · 2017-06-08T22:20:38Z

views/project/screenshots.php

@@ -59,6 +61,16 @@
                                  <i class="fa fa-times fa-stack-1x fa-inverse"></i>
                                </span>
                            </span>
+                            <span class="primary-image <?= $model->primary_image_id == $image->id ? 'hide' : '' ?>"
+                                  data-image-id="<?= $image->id ?>"
+                                  data-url="<?= Url::to(["api/1.0/projects/{$image->project->id}/primary-image"]) ?>"


URL forming is not correct. Should be:

Url::to(['/api1/project/primary-image', 'id' => $image->project->id])

samdark · 2017-06-08T22:21:10Z

views/project/screenshots.php

+                            <span class="primary-image <?= $model->primary_image_id == $image->id ? 'hide' : '' ?>"
+                                  data-image-id="<?= $image->id ?>"
+                                  data-url="<?= Url::to(["api/1.0/projects/{$image->project->id}/primary-image"]) ?>"
+                                  title="<?= Yii::t('project', 'Make as primary image') ?>">


"Use as primary image" or "Mark as primary image" or "Make this image primary".

prodex · 2017-06-09T05:33:37Z

Added fixes for all items.

samdark · 2017-06-09T06:46:52Z

modules/api1/docs.md

+
+> PUT [/projects/1](/en/api/1.0/projects)
+
+- primary_image_id: id of the primary image.


I guess more fields are available, right?

Yes, also available: 'title', 'url', 'is_opensource', 'source_url', 'yii_version', 'description', 'status', 'tagValues'.

I can add them to docs or create other a scenario for update via API.

Adding them would be good.

samdark · 2017-06-09T07:50:34Z

Merged. Thank you very much for taking care of it.

prodex added 4 commits June 5, 2017 23:47

Update primary image for project

72e0e81

fix

9455a72

Raising the primary image

aa88ac7

fix

31dee84

samdark requested changes Jun 8, 2017

View reviewed changes

prodex added 4 commits June 9, 2017 08:15

add foreign key

864be18

fix getting url to primary image

1892551

cleanup

a2c3cb4

fix

e9ccdf9

cleanup

bb07fcd

samdark requested changes Jun 9, 2017

View reviewed changes

added docs

c505e84

samdark approved these changes Jun 9, 2017

View reviewed changes

samdark merged commit 8c23cb6 into samdark:master Jun 9, 2017

prodex deleted the main-image branch June 9, 2017 07:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ability to select image as primary #42

Ability to select image as primary #42

prodex commented Jun 8, 2017

samdark left a comment

samdark Jun 8, 2017

samdark Jun 8, 2017

samdark Jun 8, 2017

samdark Jun 8, 2017

samdark Jun 8, 2017

samdark Jun 8, 2017

samdark Jun 8, 2017

samdark Jun 8, 2017

prodex Jun 9, 2017

samdark Jun 8, 2017

prodex Jun 9, 2017

samdark Jun 8, 2017

samdark Jun 8, 2017

prodex commented Jun 9, 2017

samdark Jun 9, 2017

prodex Jun 9, 2017 •

edited

Loading

samdark Jun 9, 2017

prodex Jun 9, 2017

samdark commented Jun 9, 2017

		- updatedAt: UNIX timestamp indicating when image was updated last time.


		### Update primary image <a href="#projects-update-primary-image" id="projects-update-primary-image">#</a>


		> PUT [/projects/1](/en/api/1.0/projects)

		- primary_image_id: id of the primary image.

Ability to select image as primary #42

Ability to select image as primary #42

Conversation

prodex commented Jun 8, 2017

samdark 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

prodex commented Jun 9, 2017

Choose a reason for hiding this comment

prodex Jun 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samdark commented Jun 9, 2017

prodex Jun 9, 2017 •

edited

Loading