CB-359: Implemented the reviewal of labels #264
Conversation
|
@spellew When you have time, please update this PR to fix conflicts too. Thanks! :) |
|
@ferbncode Resolved. |
| # | ||
| # You should have received a copy of the GNU General Public License along | ||
| # with this program; if not, write to the Free Software Foundation, Inc., | ||
| # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. |
There was a problem hiding this comment.
I don't think we need a license here.
There was a problem hiding this comment.
(Also for other remaining pull requests)
| import critiquebrainz.frontend.external.musicbrainz_db.exceptions as mb_exceptions | ||
| # import critiquebrainz.db.review as db_review | ||
| # from critiquebrainz.frontend.forms.rate import RatingEditForm | ||
| # from critiquebrainz.frontend.views import get_avg_rating |
There was a problem hiding this comment.
Let's remove the commented imports
There was a problem hiding this comment.
The pull request is good, but let's try to
- Fix the conflicts
- Show some data on the label page for the Label. Right now it is empty. (Atleast the name, maybe the annotations and the releases too). See, for example: https://musicbrainz.org/label/5899271b-a77a-4081-93a9-0bd05b9b4180
spellew
force-pushed
the
label-entity-support
branch
from
c531f95 to
3adc216
Compare
|
@ferbncode Updated the entity page to display some data. |
|
@spellew Right now, the information is okay, One thing I noticed for artists and labels is missing rating which is also part of the proposal. Please update this PR for including ratings. I'll add review for the code later today 👍 |
| entity_type='label', | ||
| user_id=current_user.id, | ||
| ) | ||
| my_review = my_reviews[0] if my_count else None |
There was a problem hiding this comment.
What happens if I have a review already for a label. It should be passed to the render_template to display the option of Edit Review rather than Write Review. See place.py or release_group.py for example.
There was a problem hiding this comment.
Also, other important variable used in the template like current_user.
There was a problem hiding this comment.
All the other important variables too? I think I only passed the ones that we're currently using, to the render template.
There was a problem hiding this comment.
See the base.py (the variable current_user and my_review do play a role there for displaying different button labels, for example
There was a problem hiding this comment.
I've added current_user and my_review those seem to be the only variables used outside the label template.
| @@ -53,6 +53,13 @@ | |||
| {{ _('Artist') }} | |||
| </span> | |||
| {% endif %} | |||
| {% elif entity_type == 'label' %} | |||
| <img src="{{ get_static_path('images/placeholder_place.svg') }}" {{ attributes }} /> | |||
There was a problem hiding this comment.
placeholder_disc.svg would be much relating to labels, imho.
|
@ferbncode I was planning on showing the releases, but I'm worried about time constraints, and if that can be achieved while we're working on other entities. |
|
@spellew Yes, that is my plan to do it after all the entities as an enhancement (Right now, I'll just create a ticket before merging this) |
|
Did you create a ticket already, @ferbncode ? |
|
@paramsingh I'll make sure to create it today. |
There was a problem hiding this comment.
@spellew This looks good. Let's merge this. Also, here is the ticket link: https://tickets.metabrainz.org/browse/CB-359
spellew
changed the title
| @@ -1 +1,2 @@ | |||
| ALTER TYPE entity_types ADD VALUE 'artist' AFTER 'place'; | |||
| ALTER TYPE entity_types ADD VALUE 'label' AFTER 'artist'; | |||
There was a problem hiding this comment.
I just saw this, this should be in a different script so I can run it on the main db. Generally, each PR should have a single schema change script.
There was a problem hiding this comment.
Ah, right. @spellew Please add schema change scripts to admin/schema_changes (One for artists and one for labels)
There was a problem hiding this comment.
Thank you @paramsingh for pointing that out :)
No description provided.