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

feat: Level moderation #1282

Merged
merged 12 commits into from
Jan 18, 2022
Merged

feat: Level moderation #1282

merged 12 commits into from
Jan 18, 2022

Conversation

razvan-pro
Copy link
Collaborator

@razvan-pro razvan-pro commented Jan 13, 2022

This change is Reviewable

@razvan-pro razvan-pro self-assigned this Jan 13, 2022
@razvan-pro razvan-pro changed the title Level moderation feat: Level moderation Jan 13, 2022
Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewed 17 of 17 files at r1, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @razvan-pro)


game/static/game/js/level_moderation.js, line 27 at r1 (raw file):

function confirmDelete() {
  console.log(levelID);

Do we need this console.log?


game/templates/game/level_moderation.html, line 3 at r1 (raw file):

{% extends 'game/base.html' %}
{% load static %}
{% load i18n %}

Please remove this and anything else in this file related to Crowdin and translations 🙏


game/templates/game/level_moderation.html, line 31 at r1 (raw file):

{% block nav_ocargo_moderate %}
<a class="button button--secondary button--secondary--light sub-nav--teacher-active" href="{% url 'level_moderation' %}">{% trans "Moderate" %}</a>

Like this {% trans $}


game/templates/game/level_moderation.html, line 68 at r1 (raw file):

                                    {% endfor %}
                                </ul>
            

Whitespaces


game/templates/game/level_moderation.html, line 117 at r1 (raw file):

                        <td><small>{{level.shared_with}}</small></td>
                        <td class="text-center">
                            <button class="button button--small button--primary play" value={{level.id}}>{% trans "Play" %}</button>

Remove {% trans %}


game/templates/game/level_moderation.html, line 121 at r1 (raw file):

                        <td class="text-center">
                            <button class="button button--small button--primary--danger button--icon delete" value={{level.id}}>
                                {% trans "Delete" %}<span class="iconify" data-icon="mdi:delete-outline"></span>

Remove {% trans %}


game/templates/game/level_moderation.html, line 132 at r1 (raw file):

        {% else %}
            {% if thead %}
            <p>{% trans "No levels found." %}</p>

Remove {% trans %}


game/templates/game/level_moderation.html, line 134 at r1 (raw file):

            <p>{% trans "No levels found." %}</p>
            {% else %}
            <p>{% trans "Select a class to view the levels created by students." %}</p>

Remove {% trans %}


game/tests/test_level_moderation.py, line 94 at r1 (raw file):

        )

    def create_custom_level(self, level_name):

Not blocking, but should this be in test_utils or whatever the folder is called maybe?


game/views/level_moderation.py, line 9 at r1 (raw file):

from django.http import Http404, HttpResponse
from django.shortcuts import render, get_object_or_404
from django.utils.translation import ugettext

Maybe see if you can remove this import if you can remove all ugettext from this file


game/views/level_moderation.py, line 84 at r1 (raw file):

    table_headers = [
        ugettext("Student"),

Remove all ugettext and just use the string please 🙏

Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @razvan-pro)

Copy link
Collaborator Author

@razvan-pro razvan-pro left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @razvan-pro)


game/static/game/js/level_moderation.js, line 27 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Do we need this console.log?

no 😅


game/templates/game/level_moderation.html, line 3 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Please remove this and anything else in this file related to Crowdin and translations 🙏

👍


game/templates/game/level_moderation.html, line 31 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Like this {% trans $}

Done.


game/templates/game/level_moderation.html, line 68 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Whitespaces

Done.


game/templates/game/level_moderation.html, line 117 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Remove {% trans %}

Done.


game/templates/game/level_moderation.html, line 121 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Remove {% trans %}

Done.


game/templates/game/level_moderation.html, line 132 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Remove {% trans %}

Done.


game/templates/game/level_moderation.html, line 134 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Remove {% trans %}

Done.


game/tests/test_level_moderation.py, line 94 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Not blocking, but should this be in test_utils or whatever the folder is called maybe?

Good point, I didn't know there's one already there, thank you 🙏


game/views/level_moderation.py, line 9 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Maybe see if you can remove this import if you can remove all ugettext from this file

Done.


game/views/level_moderation.py, line 84 at r1 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Remove all ugettext and just use the string please 🙏

👍

Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 4 files at r3, 1 of 1 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @razvan-pro)


game/templates/game/level_moderation.html, line 25 at r5 (raw file):

{% block header %}
<section class="banner banner--rapid-router">
    <img title="Rapid Router logo" alt="Rapid Router logo" src="{% static 'portal/img/RR_logo.svg' %}">

This should really be moved to common yeah since it creates a new circular dependency 😕


game/templates/game/level_selection.html, line 54 at r5 (raw file):

{% block header %}
<section class="banner banner--rapid-router">
    <img title="Rapid Router logo" alt="Rapid Router logo" src="{% static 'portal/img/RR_logo.svg' %}">

Same here


game/templates/game/scoreboard.html, line 35 at r5 (raw file):

{% block header %}
<section class="banner banner--rapid-router">
    <img title="Rapid Router logo" alt="Rapid Router logo" src="{% static 'portal/img/RR_logo.svg' %}">

Same here

Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewed 6 of 6 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @razvan-pro)


game/templates/game/level_moderation.html, line 25 at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

This should really be moved to common yeah since it creates a new circular dependency 😕

Looks good but I can't see the new image in common?

Copy link
Collaborator Author

@razvan-pro razvan-pro left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @faucomte97)


game/templates/game/level_moderation.html, line 25 at r5 (raw file):

Previously, faucomte97 (Florian Aucomte) wrote…

Looks good but I can't see the new image in common?

It's here: https://reviewable.io/reviews/ocadotechnology/codeforlife-portal/1723#-
🙂

Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @razvan-pro)


game/templates/game/level_moderation.html, line 25 at r5 (raw file):

Previously, razvan-pro (Razvan Mahu) wrote…

It's here: https://reviewable.io/reviews/ocadotechnology/codeforlife-portal/1723#-
🙂

Oh yeah I missed it 🤦‍♂️

Copy link
Contributor

@faucomte97 faucomte97 left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @razvan-pro)

@razvan-pro razvan-pro changed the title feat: Level moderation feat: Level moderation2 Jan 18, 2022
@razvan-pro razvan-pro changed the title feat: Level moderation2 feat: Level moderation Jan 18, 2022
@codecov
Copy link

codecov bot commented Jan 18, 2022

Codecov Report

Merging #1282 (963ca3a) into master (020ca13) will increase coverage by 0.86%.
The diff coverage is 92.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1282      +/-   ##
==========================================
+ Coverage   90.65%   91.51%   +0.86%     
==========================================
  Files          98       98              
  Lines        5317     5294      -23     
==========================================
+ Hits         4820     4845      +25     
+ Misses        497      449      -48     
Impacted Files Coverage Δ
game/views/level_moderation.py 89.47% <88.88%> (+52.80%) ⬆️
game/forms.py 100.00% <100.00%> (+22.22%) ⬆️
game/urls.py 100.00% <100.00%> (ø)
game/views/scoreboard.py 88.34% <100.00%> (-0.06%) ⬇️
game/messages.py 97.48% <0.00%> (+0.23%) ⬆️
game/permissions.py 58.82% <0.00%> (+8.23%) ⬆️
game/views/helper.py 100.00% <0.00%> (+25.00%) ⬆️

@razvan-pro razvan-pro merged commit cd3757f into master Jan 18, 2022
@razvan-pro razvan-pro deleted the level-moderation branch January 18, 2022 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants