Merge pull request #171 from IsaacZanoria/CB-263

CB-263: Update publication date of draft reviews on CritiqueBrainz when they get published

admin/schema_changes/9.sql

@@ -0,0 +1,15 @@
+BEGIN;
+
+ALTER TABLE review ADD COLUMN published_on TIMESTAMP;
+
+UPDATE review
+   SET published_on = sub.latest
+  FROM ( SELECT review_id, MAX(rv.timestamp) as latest
+           FROM review r
+           JOIN revision rv
+             ON r.id = rv.review_id
+       GROUP BY review_id
+       ) AS sub
+ WHERE is_draft = 'f' AND sub.review_id = id;
+
+COMMIT;

admin/sql/create_tables.sql

@@ -49,19 +49,22 @@ ALTER TABLE oauth_token ADD CONSTRAINT oauth_token_access_token_key UNIQUE (acce
 ALTER TABLE oauth_token ADD CONSTRAINT oauth_token_refresh_token_key UNIQUE (refresh_token);
 
 CREATE TABLE review (
-    id          UUID         NOT NULL DEFAULT uuid_generate_v4(),
-    entity_id   UUID         NOT NULL,
-    entity_type entity_types NOT NULL,
-    user_id     UUID         NOT NULL,
-    edits       INTEGER      NOT NULL,
-    is_draft    BOOLEAN      NOT NULL,
-    is_hidden   BOOLEAN      NOT NULL,
-    license_id  VARCHAR      NOT NULL,
-    language    VARCHAR(3)   NOT NULL,
-    source      VARCHAR,
-    source_url  VARCHAR
+    id              UUID         NOT NULL DEFAULT uuid_generate_v4(),
+    entity_id       UUID         NOT NULL,
+    entity_type     entity_types NOT NULL,
+    user_id         UUID         NOT NULL,
+    edits           INTEGER      NOT NULL,
+    is_draft        BOOLEAN      NOT NULL,
+    is_hidden       BOOLEAN      NOT NULL,
+    license_id      VARCHAR      NOT NULL,
+    language        VARCHAR(3)   NOT NULL,
+    published_on    TIMESTAMP,
+    source          VARCHAR,
+    source_url      VARCHAR
 );
 ALTER TABLE review ADD CONSTRAINT review_entity_id_user_id_key UNIQUE (entity_id, user_id);
+ALTER TABLE review ADD CONSTRAINT published_on_null_for_drafts_and_not_null_for_published_reviews
+    CHECK ((is_draft = 't' AND published_on IS NULL) OR (is_draft = 'f' And published_on IS NOT NULL));
 
 CREATE TABLE revision (
     id          SERIAL      NOT NULL,

critiquebrainz/data/dump_manager.py

@@ -30,6 +30,7 @@
         "is_hidden",
         "license_id",
         "language",
+        "published_on",
         "source",
         "source_url",
     ),

critiquebrainz/db/__init__.py

@@ -2,7 +2,7 @@
 from sqlalchemy.pool import NullPool
 
 # This value must be incremented after schema changes on exported tables!
-SCHEMA_VERSION = 8
+SCHEMA_VERSION = 9
 VALID_RATING_VALUES = [None, 1, 2, 3, 4, 5]
 REVIEW_RATING_MAX = 5
 REVIEW_RATING_MIN = 1

critiquebrainz/db/review.py

@@ -66,8 +66,8 @@ def get_by_id(review_id):
             "popularity": int,
             "rating": int,
             "text": str,
-            "created": datetime,
             "license": dict,
+            "published_on": datetime,
         }
     """
     with db.engine.connect() as connection:
@@ -83,6 +83,7 @@ def get_by_id(review_id):
                    review.language,
                    review.source,
                    review.source_url,
+                   review.published_on,
                    revision.id AS last_revision_id,
                    revision.timestamp,
                    revision.text,
@@ -93,23 +94,13 @@ def get_by_id(review_id):
                    "user".show_gravatar,
                    "user".musicbrainz_id,
                    "user".is_blocked,
-                   created_time.created,
                    license.full_name,
                    license.info_url
               FROM review
               JOIN revision ON revision.review_id = review.id
               JOIN "user" ON "user".id = review.user_id
               JOIN license ON license.id = license_id
-              JOIN (
-                    SELECT review.id,
-                           timestamp AS created
-                      FROM review
-                      JOIN revision ON review.id = revision.review_id
-                     WHERE review.id = :review_id
-                  ORDER BY revision.timestamp ASC
-                     LIMIT 1
-                   ) AS created_time
-                ON created_time.id = review.id
+             WHERE review.id = :review_id
           ORDER BY timestamp DESC
         """), {
             "review_id": review_id,
@@ -220,6 +211,10 @@ def update(review_id, *, drafted, text=None, rating=None, license_id=None, langu
     if is_draft is not None:
         if not drafted and is_draft:  # If trying to convert published review into draft
             raise db_exceptions.BadDataException("Converting published reviews back to drafts is not allowed.")
+        if drafted and not is_draft:
+            published_on = datetime.now()
+            updates.append("published_on = :published_on")
+            updated_info["published_on"] = published_on
         updates.append("is_draft = :is_draft")
         updated_info["is_draft"] = is_draft
 
@@ -281,19 +276,23 @@ def create(*, entity_id, entity_type, user_id, is_draft, text=None, rating=None,
             "popularity": int,
             "text": str,
             "rating": int,
-            "created": datetime,
             "license": dict,
+            "published_on": datetime,
         }
     """
     if text is None and rating is None:
         raise db_exceptions.BadDataException("Text part and rating part of a review can not be None simultaneously")
     if language not in supported_languages:
         raise ValueError("Language: {} is not supported".format(language))
+    if is_draft:
+        published_on = None
+    else:
+        published_on = datetime.now()
 
     with db.engine.connect() as connection:
         result = connection.execute(sqlalchemy.text("""
-            INSERT INTO review (id, entity_id, entity_type, user_id, edits, is_draft, is_hidden, license_id, language, source, source_url)
-            VALUES (:id, :entity_id, :entity_type, :user_id, :edits, :is_draft, :is_hidden, :license_id, :language, :source, :source_url)
+            INSERT INTO review (id, entity_id, entity_type, user_id, edits, is_draft, is_hidden, license_id, language, source, source_url, published_on)
+            VALUES (:id, :entity_id, :entity_type, :user_id, :edits, :is_draft, :is_hidden, :license_id, :language, :source, :source_url, :published_on)
          RETURNING id;
         """), {  # noqa: E501
             "id": str(uuid.uuid4()),
@@ -307,6 +306,7 @@ def create(*, entity_id, entity_type, user_id, is_draft, text=None, rating=None,
             "license_id": license_id,
             "source": source,
             "source_url": source_url,
+            "published_on": published_on,
         })
         review_id = result.fetchone()[0]
         # TODO(roman): It would be better to create review and revision in one transaction
@@ -329,7 +329,7 @@ def list_reviews(*, inc_drafts=False, inc_hidden=False, entity_id=None,
         entity_type (str): Type of the entity that has been reviewed.
         user_id (uuid): ID of the author.
         sort (str): Order of the returned reviews. Can be either "popularity" (order by difference in +/- votes),
-                    or "created" (order by creation time), or "random" (order randomly).
+                    or "published_on" (order by publish time), or "random" (order randomly).
         limit (int): Maximum number of reviews to return.
         offset (int): Offset that can be used in conjunction with the limit.
         language (str): Language code of reviews.
@@ -397,9 +397,9 @@ def list_reviews(*, inc_drafts=False, inc_hidden=False, entity_id=None,
         order_by_clause = """
             ORDER BY popularity DESC
         """
-    elif sort == "created":
+    elif sort == "published_on":
         order_by_clause = """
-            ORDER BY created DESC
+            ORDER BY review.published_on DESC
         """
     elif sort == "random":
         order_by_clause = """
@@ -424,6 +424,7 @@ def list_reviews(*, inc_drafts=False, inc_hidden=False, entity_id=None,
                "user".email,
                "user".created as user_created,
                "user".musicbrainz_id,
+               review.published_on,
                MIN(revision.timestamp) as created,
                SUM(
                    CASE WHEN vote='t' THEN 1 ELSE 0 END

critiquebrainz/db/review_test.py

@@ -196,7 +196,7 @@ def test_list_reviews(self):
         self.assertEqual(count, 1)
         self.assertEqual(len(reviews), 1)
 
-        reviews, count = db_review.list_reviews(sort="created")
+        reviews, count = db_review.list_reviews(sort="published_on")
         self.assertEqual(count, 1)
         self.assertEqual(len(reviews), 1)
 

critiquebrainz/frontend/templates/event/entity.html

@@ -77,7 +77,7 @@ <h4 style="margin-bottom:0;">{{ _('Reviews') }}</h4>
                   {{ _('by %(reviewer)s', reviewer='<img class="avatar" src="%s&s=16" /> '|safe % review.user.avatar + review.user.display_name) }}
                 </a>
               </td>
-              <td>{{ review.created | date }}</td>
+              <td>{{ review.published_on | date }}</td>
               <td>{{ review.votes_positive_count }}/{{ review.votes_negative_count }}</td>
             </tr>
           {% endfor %}

critiquebrainz/frontend/templates/place/entity.html

@@ -68,7 +68,7 @@ <h4 style="margin-bottom:0;">{{ _('Reviews') }}</h4>
                   {{ _('by %(reviewer)s', reviewer='<img class="avatar" src="%s&s=16" /> '|safe % review.user.avatar + review.user.display_name) }}
                 </a>
               </td>
-              <td>{{ review.created | date }}</td>
+              <td>{{ review.published_on | date }}</td>
               <td>{{ review.votes_positive_count }}/{{ review.votes_negative_count }}</td>
             </tr>
           {% endfor %}

critiquebrainz/frontend/templates/release_group/entity.html

@@ -67,7 +67,7 @@ <h4 style="margin-bottom:0;">{{ _('Reviews') }}</h4>
                   {{ _('by %(reviewer)s', reviewer='<img class="avatar" src="%s&s=16" /> '|safe % review.user.avatar + review.user.display_name) }}
                 </a>
               </td>
-              <td>{{ review.created | date }}</td>
+              <td>{{ review.published_on | date }}</td>
               <td>{{ review.votes_positive_count }}/{{ review.votes_negative_count }}</td>
             </tr>
           {% endfor %}

critiquebrainz/frontend/templates/review/browse.html

@@ -34,7 +34,7 @@ <h2>{{ _('Reviews') }}</h2>
         <p>
           {{ review_credit(review, user_picture_size=16) }}
           <br/>
-          {{ review.created | date }}
+          {{ review.published_on | date }}
         </p>
       </div>
     </div>

critiquebrainz/frontend/templates/review/entity/base.html

@@ -17,7 +17,7 @@ <h3>{{ review_credit(review, user_picture_size=24) }}</h3>
       {% if review.is_draft %}
         <div class="alert alert-warning" role="alert">{{ _('<b>This review has not been published yet!</b> Only you can see it.') }}</div>
       {% else %}
-        <em class="text-muted">{{ _('Published on %(date)s', date = review.created|date) }}</em><br />
+        <em class="text-muted">{{ _('Published on %(date)s', date = review.published_on|date) }}</em><br />
         {% if review.text %}
           <em class="text-muted">{{ _('Written in %(language)s', language = review.language|language_name) }}</em><br />
           {% set votes_total = review.votes.positive + review.votes.negative %}
@@ -135,7 +135,7 @@ <h3>
               </div>
               {% set entity = review.entity_id | entity_details(type=review.entity_type) %}
               <div class="album">{% include 'entity_review.html' %}</div>
-              {{ review.created | date }}
+              {{ review.published_on | date }}
             </div>
           {% endfor %}
         </div>

critiquebrainz/frontend/templates/user/reviews.html

@@ -14,6 +14,7 @@
           <th>{{ _('Status') }}</th>
         {% endif %}
         <th>{{ _('Created on') }}</th>
+        <th>{{ _('Published on')}} </th>
         <th>{{ _('Votes (+/-)') }}</th>
         {% if current_user.is_authenticated and current_user == user %}
           <th><!-- Buttons --></th>
@@ -71,6 +72,11 @@
             </td>
           {% endif %}
           <td>{{ review.created | date }}</td>
+          {% if review.is_draft %}
+            <td> Not published yet! </td>
+          {% else %}
+            <td>{{ review.published_on | date }}</td>
+          {% endif %}
           <td>{{ review.votes_positive_count }}/{{ review.votes_negative_count }}</td>
           {% if current_user.is_authenticated and current_user == user %}
             <td>

critiquebrainz/frontend/views/artist.py

@@ -45,7 +45,7 @@ def entity(mbid):
         reviews, review_count = db_review.list_reviews(  # pylint: disable=unused-variable
             entity_id=release_group['id'],
             entity_type='release_group',
-            sort='created', limit=1,
+            sort='published_on', limit=1,
         )
         release_group['review_count'] = review_count
 

critiquebrainz/frontend/views/index.py

@@ -20,7 +20,7 @@ def index():
         review['preview'] = ''.join(BeautifulSoup(preview, "html.parser").findAll(text=True))
 
     # Recent reviews
-    recent_reviews, _ = db_review.list_reviews(sort='created', limit=9)
+    recent_reviews, _ = db_review.list_reviews(sort='published_on', limit=9)
 
     # Statistics
     review_count = format_number(db_review.get_count(is_draft=False))

critiquebrainz/frontend/views/user.py

@@ -29,7 +29,7 @@ def reviews(user_id):
         return redirect(url_for('.reviews'))
     limit = 12
     offset = (page - 1) * limit
-    reviews, count = db_review.list_reviews(user_id=user_id, sort='created', limit=limit, offset=offset,
+    reviews, count = db_review.list_reviews(user_id=user_id, sort='published_on', limit=limit, offset=offset,
                                             inc_hidden=current_user.is_admin(),
                                             inc_drafts=current_user.is_authenticated and current_user.id == user_id)
     return render_template('user/reviews.html', section='reviews', user=user,