diff --git a/.github/actions/compile_messages/action.yml b/.github/actions/compile_messages/action.yml new file mode 100644 index 00000000..aead1e77 --- /dev/null +++ b/.github/actions/compile_messages/action.yml @@ -0,0 +1,8 @@ +name: "Compile messages" +description: "Compile the gettext translation messages" +runs: + using: composite + steps: + - name: Setup project + run: poetry run ./manage.py compilemessages + shell: bash \ No newline at end of file diff --git a/.github/actions/setup_project/action.yml b/.github/actions/setup_project/action.yml new file mode 100644 index 00000000..b1eb6e2a --- /dev/null +++ b/.github/actions/setup_project/action.yml @@ -0,0 +1,53 @@ +name: "Setup project" +description: "Setup Python and Poetry" +runs: + using: composite + steps: + - name: Install apt packages + uses: awalsh128/cache-apt-pkgs-action@latest + with: + packages: gettext libxapian-dev libgraphviz-dev + version: 1.0 # increment to reset cache + + - name: Install dependencies + run: | + sudo apt update + sudo apt install gettext libxapian-dev libgraphviz-dev + shell: bash + + - name: Set up python + uses: actions/setup-python@v4 + with: + python-version: "3.10" + + - name: Load cached Poetry installation + id: cached-poetry + uses: actions/cache@v3 + with: + path: ~/.local + key: poetry-0 # increment to reset cache + + - name: Install Poetry + if: steps.cached-poetry.outputs.cache-hit != 'true' + shell: bash + run: curl -sSL https://install.python-poetry.org | python3 - + + - name: Check pyproject.toml syntax + shell: bash + run: poetry check + + - name: Load cached dependencies + uses: actions/cache@v3 + with: + path: ~/.cache/pypoetry + key: ${{ runner.os }}-poetry-${{ hashFiles('**/poetry.lock') }} + restore-keys: | + ${{ runner.os }}-poetry- + + - name: Install dependencies + run: poetry install -E testing -E docs + shell: bash + + - name: Compile gettext messages + run: poetry run ./manage.py compilemessages + shell: bash diff --git a/.github/actions/setup_xapian/action.yml b/.github/actions/setup_xapian/action.yml new file mode 100644 index 00000000..6678f581 --- /dev/null +++ b/.github/actions/setup_xapian/action.yml @@ -0,0 +1,10 @@ +name: "Setup xapian" +description: "Setup the xapian indexes" +runs: + using: composite + steps: + - name: Setup xapian index + run: | + mkdir -p /dev/shm/search_indexes + ln -s /dev/shm/search_indexes sith/search_indexes + shell: bash diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 00000000..da2bf91f --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,43 @@ +name: Sith 3 CI + +on: + push: + branches: + - master + - taiste + pull_request: + branches: + - master + - taiste + +jobs: + black: + name: Black format + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v3 + - name: Setup Project + uses: ./.github/actions/setup_project + - run: poetry run black --check . + + tests: + name: Run tests and generate coverage report + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v3 + - uses: ./.github/actions/setup_project + - uses: ./.github/actions/setup_xapian + - uses: ./.github/actions/compile_messages + - name: Run tests + run: poetry run coverage run ./manage.py test + - name: Generate coverage report + run: | + poetry run coverage report + poetry run coverage html + - name: Archive code coverage results + uses: actions/upload-artifact@v3 + with: + name: coverage-report + path: coverage_report diff --git a/.github/workflows/unittests.yml b/.github/workflows/unittests.yml deleted file mode 100644 index 9527774a..00000000 --- a/.github/workflows/unittests.yml +++ /dev/null @@ -1,83 +0,0 @@ -name: Sith3 CI - -on: - pull_request: - branches: [ master, taiste ] - push: - branches: [ master, taiste ] - -jobs: - unittests: - runs-on: ubuntu-latest - timeout-minutes: 30 - - steps: - - uses: actions/checkout@v3 - - # Skip unit testing if no diff on .py files - - name: Check file diff - uses: technote-space/get-diff-action@v6 - id: git-diff - with: - PATTERNS: | - **/*.py - - - name: Set up python - if: steps.git-diff.outputs.diff - uses: actions/setup-python@v4 - with: - python-version: '3.8' - - - name: Install dependencies - if: steps.git-diff.outputs.diff - run: | - sudo apt-get update - sudo apt-get install gettext libxapian-dev libgraphviz-dev - - - name: Install poetry - if: steps.git-diff.outputs.diff - run: | - python -m pip install --upgrade pip - python -m pip install poetry - - - name: Checking pyproject.toml syntax - if: steps.git-diff.outputs.diff - run: poetry check - - - name: Install project - if: steps.git-diff.outputs.diff - run: poetry install -E testing - - - name: Setup xapian index - if: steps.git-diff.outputs.diff - run: | - mkdir -p /dev/shm/search_indexes - ln -s /dev/shm/search_indexes sith/search_indexes - - - name: Setup project - if: steps.git-diff.outputs.diff - run: poetry run ./manage.py compilemessages - - - name: Launch tests and generate coverage report - if: steps.git-diff.outputs.diff - run: | - poetry run coverage run ./manage.py test - poetry run coverage report - - lint: - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v3 - - name: Set up python - uses: actions/setup-python@v4 - with: - python-version: '3.8' - - - name: Install black - run: | - python -m pip install --upgrade pip - python -m pip install black==22.6.0 - - - name: Check linting - run: black --check . \ No newline at end of file diff --git a/accounting/migrations/0001_initial.py b/accounting/migrations/0001_initial.py index 0196d3f2..51add331 100644 --- a/accounting/migrations/0001_initial.py +++ b/accounting/migrations/0001_initial.py @@ -8,7 +8,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [] operations = [ diff --git a/accounting/migrations/0002_auto_20160824_2152.py b/accounting/migrations/0002_auto_20160824_2152.py index d3b1c9a2..b0cc051f 100644 --- a/accounting/migrations/0002_auto_20160824_2152.py +++ b/accounting/migrations/0002_auto_20160824_2152.py @@ -6,7 +6,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [ ("club", "0001_initial"), ("accounting", "0001_initial"), diff --git a/accounting/migrations/0003_auto_20160824_2203.py b/accounting/migrations/0003_auto_20160824_2203.py index 47ae232f..597f582f 100644 --- a/accounting/migrations/0003_auto_20160824_2203.py +++ b/accounting/migrations/0003_auto_20160824_2203.py @@ -6,7 +6,6 @@ import phonenumber_field.modelfields class Migration(migrations.Migration): - dependencies = [("accounting", "0002_auto_20160824_2152")] operations = [ diff --git a/accounting/migrations/0004_auto_20161005_1505.py b/accounting/migrations/0004_auto_20161005_1505.py index 8fe3a472..993754bd 100644 --- a/accounting/migrations/0004_auto_20161005_1505.py +++ b/accounting/migrations/0004_auto_20161005_1505.py @@ -6,7 +6,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [("accounting", "0003_auto_20160824_2203")] operations = [ diff --git a/accounting/migrations/0005_auto_20170324_0917.py b/accounting/migrations/0005_auto_20170324_0917.py index c8258f8e..76bcbed6 100644 --- a/accounting/migrations/0005_auto_20170324_0917.py +++ b/accounting/migrations/0005_auto_20170324_0917.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("accounting", "0004_auto_20161005_1505")] operations = [ diff --git a/accounting/models.py b/accounting/models.py index 8579e436..e10a029d 100644 --- a/accounting/models.py +++ b/accounting/models.py @@ -66,7 +66,7 @@ class Company(models.Model): """ Method to see if that object can be edited by the given user """ - if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): + if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): return True return False @@ -117,7 +117,9 @@ class BankAccount(models.Model): """ Method to see if that object can be edited by the given user """ - if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): + if user.is_anonymous: + return False + if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): return True m = self.club.get_membership_for(user) if m is not None and m.role >= settings.SITH_CLUB_ROLES_ID["Treasurer"]: @@ -154,7 +156,9 @@ class ClubAccount(models.Model): """ Method to see if that object can be edited by the given user """ - if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): + if user.is_anonymous: + return False + if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): return True return False @@ -225,7 +229,9 @@ class GeneralJournal(models.Model): """ Method to see if that object can be edited by the given user """ - if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): + if user.is_anonymous: + return False + if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): return True if self.club_account.can_be_edited_by(user): return True @@ -235,7 +241,7 @@ class GeneralJournal(models.Model): """ Method to see if that object can be edited by the given user """ - if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): + if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): return True if self.club_account.can_be_edited_by(user): return True @@ -414,7 +420,9 @@ class Operation(models.Model): """ Method to see if that object can be edited by the given user """ - if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): + if user.is_anonymous: + return False + if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): return True if self.journal.closed: return False @@ -427,7 +435,7 @@ class Operation(models.Model): """ Method to see if that object can be edited by the given user """ - if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): + if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): return True if self.journal.closed: return False @@ -483,7 +491,9 @@ class AccountingType(models.Model): """ Method to see if that object can be edited by the given user """ - if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): + if user.is_anonymous: + return False + if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): return True return False @@ -554,6 +564,8 @@ class Label(models.Model): ) def is_owned_by(self, user): + if user.is_anonymous: + return False return self.club_account.is_owned_by(user) def can_be_edited_by(self, user): diff --git a/accounting/templates/accounting/bank_account_details.jinja b/accounting/templates/accounting/bank_account_details.jinja index 9ab06c99..eb7ac8b1 100644 --- a/accounting/templates/accounting/bank_account_details.jinja +++ b/accounting/templates/accounting/bank_account_details.jinja @@ -12,7 +12,7 @@


{% trans %}Bank account: {% endtrans %}{{ object.name }}

- {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) and not object.club_accounts.exists() %} + {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) and not object.club_accounts.exists() %} {% trans %}Delete{% endtrans %} {% endif %}

{% trans %}Infos{% endtrans %}

diff --git a/accounting/templates/accounting/bank_account_list.jinja b/accounting/templates/accounting/bank_account_list.jinja index 0fcab4a5..0bd654b8 100644 --- a/accounting/templates/accounting/bank_account_list.jinja +++ b/accounting/templates/accounting/bank_account_list.jinja @@ -9,7 +9,7 @@

{% trans %}Accounting{% endtrans %}

- {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %} + {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}

{% trans %}Manage simplified types{% endtrans %}

{% trans %}Manage accounting types{% endtrans %}

{% trans %}New bank account{% endtrans %}

diff --git a/accounting/templates/accounting/club_account_details.jinja b/accounting/templates/accounting/club_account_details.jinja index 13626950..f9ceec23 100644 --- a/accounting/templates/accounting/club_account_details.jinja +++ b/accounting/templates/accounting/club_account_details.jinja @@ -16,7 +16,7 @@ {% if user.is_root and not object.journals.exists() %} {% trans %}Delete{% endtrans %} {% endif %} - {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %} + {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}

{% trans %}New label{% endtrans %}

{% endif %}

{% trans %}Label list{% endtrans %}

@@ -56,7 +56,7 @@ {% endif %} {% trans %}View{% endtrans %} {% trans %}Edit{% endtrans %} - {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) and j.operations.count() == 0 %} + {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) and j.operations.count() == 0 %} {% trans %}Delete{% endtrans %} {% endif %} diff --git a/accounting/templates/accounting/co_list.jinja b/accounting/templates/accounting/co_list.jinja index 85b61c2c..0cca32c6 100644 --- a/accounting/templates/accounting/co_list.jinja +++ b/accounting/templates/accounting/co_list.jinja @@ -6,11 +6,12 @@ {% block content %}
- {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) or user.is_root %} + {% if user.is_root + or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) + %}

{% trans %}Create new company{% endtrans %}

{% endif %} - -
+
diff --git a/accounting/templates/accounting/journal_details.jinja b/accounting/templates/accounting/journal_details.jinja index 2e76f39d..274e678f 100644 --- a/accounting/templates/accounting/journal_details.jinja +++ b/accounting/templates/accounting/journal_details.jinja @@ -84,10 +84,13 @@ {% endif %} diff --git a/accounting/templates/accounting/label_list.jinja b/accounting/templates/accounting/label_list.jinja index f790c608..c15a9373 100644 --- a/accounting/templates/accounting/label_list.jinja +++ b/accounting/templates/accounting/label_list.jinja @@ -13,7 +13,7 @@


{% trans %}Back to club account{% endtrans %}

- {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %} + {% if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}

{% trans %}New label{% endtrans %}

{% endif %} {% if object.labels.all() %} @@ -21,7 +21,7 @@
- - {% if o.journal.club_account.bank_account.name != "AE TI" and o.journal.club_account.bank_account.name != "TI" or user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %} - {% if not o.journal.closed %} - {% trans %}Edit{% endtrans %} - {% endif %} + {% + if o.journal.club_account.bank_account.name not in ["AE TI", "TI"] + or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) + %} + {% if not o.journal.closed %} + {% trans %}Edit{% endtrans %} + {% endif %} {% endif %} {% trans %}Generate{% endtrans %}
- - - - - {% if users_old %} - - {% endif %} + + + + + + {% if users_old %} + + {% endif %} + {% for m in members %} diff --git a/club/tests.py b/club/tests.py index e6df319b..0cc05e89 100644 --- a/club/tests.py +++ b/club/tests.py @@ -13,378 +13,564 @@ # OR WITHIN THE LOCAL FILE "LICENSE" # # +from datetime import timedelta from django.conf import settings +from django.core.cache import cache from django.test import TestCase from django.utils import timezone, html +from django.utils.timezone import now, localtime from django.utils.translation import gettext as _ from django.urls import reverse from django.core.management import call_command -from django.core.exceptions import ValidationError, NON_FIELD_ERRORS -from core.models import User +from core.models import User, AnonymousUser from club.models import Club, Membership, Mailing from club.forms import MailingForm -from sith.settings import SITH_BAR_MANAGER - - -# Create your tests here. +from sith.settings import SITH_BAR_MANAGER, SITH_MAIN_CLUB_ID class ClubTest(TestCase): + """ + Set up data for test cases related to clubs and membership + The generated dataset is the one created by the populate command, + plus the following modifications : + + - `self.club` is a dummy club recreated for each test + - `self.club` has two board members : skia (role 3) and comptable (role 10) + - `self.club` has one regular member : richard + - `self.club` has one former member : sli (who had role 2) + - None of the `self.club` members are in the AE club. + """ + + @classmethod + def setUpTestData(cls): + # subscribed users - initial members + cls.skia = User.objects.get(username="skia") + cls.richard = User.objects.get(username="rbatsbak") + cls.comptable = User.objects.get(username="comptable") + cls.sli = User.objects.get(username="sli") + + # subscribed users - not initial members + cls.krophil = User.objects.get(username="krophil") + cls.subscriber = User.objects.get(username="subscriber") + + # old subscriber + cls.old_subscriber = User.objects.get(username="old_subscriber") + + # not subscribed + cls.public = User.objects.get(username="public") + + cls.ae = Club.objects.filter(pk=SITH_MAIN_CLUB_ID)[0] + def setUp(self): - call_command("populate") - self.skia = User.objects.filter(username="skia").first() - self.rbatsbak = User.objects.filter(username="rbatsbak").first() - self.guy = User.objects.filter(username="guy").first() - self.bdf = Club.objects.filter(unix_name=SITH_BAR_MANAGER["unix_name"]).first() + # by default, Skia is in the AE, which creates side effect + self.skia.memberships.all().delete() - def test_create_add_user_to_club_from_root_ok(self): - self.client.login(username="root", password="plop") - self.client.post( - reverse("club:club_members", kwargs={"club_id": self.bdf.id}), - {"users": self.skia.id, "start_date": "12/06/2016", "role": 3}, + # create a fake club + self.club = Club.objects.create( + name="Fake Club", + unix_name="fake-club", + address="5 rue de la République, 90000 Belfort", ) - response = self.client.get( - reverse("club:club_members", kwargs={"club_id": self.bdf.id}) + self.members_url = reverse( + "club:club_members", kwargs={"club_id": self.club.id} ) - self.assertTrue(response.status_code == 200) - self.assertTrue( - "S' Kia\\n " - in str(response.content) + a_month_ago = now() - timedelta(days=30) + yesterday = now() - timedelta(days=1) + Membership.objects.create( + club=self.club, user=self.skia, start_date=a_month_ago, role=3 + ) + Membership.objects.create(club=self.club, user=self.richard, role=1) + Membership.objects.create( + club=self.club, user=self.comptable, start_date=a_month_ago, role=10 ) - def test_create_add_multiple_user_to_club_from_root_ok(self): + # sli was a member but isn't anymore + Membership.objects.create( + club=self.club, + user=self.sli, + start_date=a_month_ago, + end_date=yesterday, + role=2, + ) + cache.clear() + + +class MembershipQuerySetTest(ClubTest): + def test_ongoing(self): + """ + Test that the ongoing queryset method returns the memberships that + are not ended. + """ + current_members = self.club.members.ongoing() + expected = [ + self.skia.memberships.get(club=self.club), + self.comptable.memberships.get(club=self.club), + self.richard.memberships.get(club=self.club), + ] + self.assertEqual(len(current_members), len(expected)) + for member in current_members: + self.assertIn(member, expected) + + def test_board(self): + """ + Test that the board queryset method returns the memberships + of user in the club board + """ + board_members = list(self.club.members.board()) + expected = [ + self.skia.memberships.get(club=self.club), + self.comptable.memberships.get(club=self.club), + # sli is no more member, but he was in the board + self.sli.memberships.get(club=self.club), + ] + self.assertEqual(len(board_members), len(expected)) + for member in board_members: + self.assertIn(member, expected) + + def test_ongoing_board(self): + """ + Test that combining ongoing and board returns users + who are currently board members of the club + """ + members = list(self.club.members.ongoing().board()) + expected = [ + self.skia.memberships.get(club=self.club), + self.comptable.memberships.get(club=self.club), + ] + self.assertEqual(len(members), len(expected)) + for member in members: + self.assertIn(member, expected) + + def test_update_invalidate_cache(self): + """ + Test that the `update` queryset method properly invalidate cache + """ + mem_skia = self.skia.memberships.get(club=self.club) + cache.set(f"membership_{mem_skia.club_id}_{mem_skia.user_id}", mem_skia) + self.skia.memberships.update(end_date=localtime(now()).date()) + self.assertEqual( + cache.get(f"membership_{mem_skia.club_id}_{mem_skia.user_id}"), "not_member" + ) + + mem_richard = self.richard.memberships.get(club=self.club) + cache.set( + f"membership_{mem_richard.club_id}_{mem_richard.user_id}", mem_richard + ) + self.richard.memberships.update(role=5) + new_mem = self.richard.memberships.get(club=self.club) + self.assertNotEqual(new_mem, "not_member") + self.assertEqual(new_mem.role, 5) + + def test_delete_invalidate_cache(self): + """ + Test that the `delete` queryset properly invalidate cache + """ + + mem_skia = self.skia.memberships.get(club=self.club) + mem_comptable = self.comptable.memberships.get(club=self.club) + cache.set(f"membership_{mem_skia.club_id}_{mem_skia.user_id}", mem_skia) + cache.set( + f"membership_{mem_comptable.club_id}_{mem_comptable.user_id}", mem_comptable + ) + + # should delete the subscriptions of skia and comptable + self.club.members.ongoing().board().delete() + + self.assertEqual( + cache.get(f"membership_{mem_skia.club_id}_{mem_skia.user_id}"), "not_member" + ) + self.assertEqual( + cache.get(f"membership_{mem_comptable.club_id}_{mem_comptable.user_id}"), + "not_member", + ) + + +class ClubModelTest(ClubTest): + def assert_membership_just_started(self, user: User, role: int): + """ + Assert that the given membership is active and started today + """ + membership = user.memberships.ongoing().filter(club=self.club).first() + self.assertIsNotNone(membership) + self.assertEqual(localtime(now()).date(), membership.start_date) + self.assertIsNone(membership.end_date) + self.assertEqual(membership.role, role) + self.assertEqual(membership.club.get_membership_for(user), membership) + member_group = self.club.unix_name + settings.SITH_MEMBER_SUFFIX + board_group = self.club.unix_name + settings.SITH_BOARD_SUFFIX + self.assertTrue(user.is_in_group(name=member_group)) + self.assertTrue(user.is_in_group(name=board_group)) + + def assert_membership_just_ended(self, user: User): + """ + Assert that the given user have a membership which ended today + """ + today = localtime(now()).date() + self.assertIsNotNone( + user.memberships.filter(club=self.club, end_date=today).first() + ) + self.assertIsNone(self.club.get_membership_for(user)) + + def test_access_unauthorized(self): + """ + Test that users who never subscribed and anonymous users + cannot see the page + """ + response = self.client.post(self.members_url) + self.assertEqual(response.status_code, 403) + + self.client.login(username="public", password="plop") + response = self.client.post(self.members_url) + self.assertEqual(response.status_code, 403) + + def test_display(self): + """ + Test that a GET request return a page where the requested + information are displayed. + """ + self.client.login(username=self.skia.username, password="plop") + response = self.client.get(self.members_url) + self.assertEqual(response.status_code, 200) + expected_html = ( + "
{% trans %}User{% endtrans %}{% trans %}Role{% endtrans %}{% trans %}Description{% endtrans %}{% trans %}Since{% endtrans %}{% trans %}Mark as old{% endtrans %}
{% trans %}User{% endtrans %}{% trans %}Role{% endtrans %}{% trans %}Description{% endtrans %}{% trans %}Since{% endtrans %}{% trans %}Mark as old{% endtrans %}
Responsable info
" + "" + "" + "" + ) + memberships = self.club.members.ongoing().order_by("-role") + input_id = 0 + for membership in memberships.select_related("user"): + user = membership.user + expected_html += ( + f"" + f"" + f"" + f"" + expected_html += "
UtilisateurRôleDescriptionDepuisMarquer comme ancien
" + f"{user.get_display_name()}{settings.SITH_CLUB_ROLES[membership.role]}{membership.description}{membership.start_date}" + ) + if membership.role <= 3: # 3 is the role of skia + expected_html += ( + '' + ) + input_id += 1 + expected_html += "
" + self.assertInHTML(expected_html, response.content.decode()) + + def test_root_add_one_club_member(self): + """ + Test that root users can add members to clubs, one at a time + """ self.client.login(username="root", password="plop") - self.client.post( - reverse("club:club_members", kwargs={"club_id": self.bdf.id}), + response = self.client.post( + self.members_url, + {"users": self.subscriber.id, "role": 3}, + ) + self.assertRedirects(response, self.members_url) + self.subscriber.refresh_from_db() + self.assert_membership_just_started(self.subscriber, role=3) + + def test_root_add_multiple_club_member(self): + """ + Test that root users can add multiple members at once to clubs + """ + self.client.login(username="root", password="plop") + response = self.client.post( + self.members_url, { - "users": "|%d|%d|" % (self.skia.id, self.rbatsbak.id), - "start_date": "12/06/2016", + "users": f"|{self.subscriber.id}|{self.krophil.id}|", "role": 3, }, ) - response = self.client.get( - reverse("club:club_members", kwargs={"club_id": self.bdf.id}) - ) - self.assertTrue(response.status_code == 200) - content = str(response.content) - self.assertTrue( - "S' Kia\\n Responsable info" - in content - ) - self.assertTrue( - "Richard Batsbak\\n Responsable info" - in content - ) + self.assertRedirects(response, self.members_url) + self.subscriber.refresh_from_db() + self.assert_membership_just_started(self.subscriber, role=3) + self.assert_membership_just_started(self.krophil, role=3) - def test_create_add_user_to_club_from_root_fail_not_subscriber(self): + def test_add_unauthorized_members(self): + """ + Test that users who are not currently subscribed + cannot be members of clubs. + """ self.client.login(username="root", password="plop") response = self.client.post( - reverse("club:club_members", kwargs={"club_id": self.bdf.id}), - {"users": self.guy.id, "start_date": "12/06/2016", "role": 3}, + self.members_url, + {"users": self.public.id, "role": 1}, ) - self.assertTrue(response.status_code == 200) + self.assertIsNone(self.public.memberships.filter(club=self.club).first()) self.assertTrue('
- {% if user.memberships.filter(end_date=None).exists() or user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) or user == profile or user.is_in_group(settings.SITH_BAR_MANAGER_BOARD_GROUP) %} + {% if + user == profile + or user.memberships.ongoing().exists() + or user.is_board_member + or user.is_in_group(name=settings.SITH_BAR_MANAGER_BOARD_GROUP) + %} {# if the user is member of a club, he can view the subscription state #}
{% if profile.is_subscribed %} diff --git a/core/templates/core/user_edit.jinja b/core/templates/core/user_edit.jinja index 1bac52dc..f189d339 100644 --- a/core/templates/core/user_edit.jinja +++ b/core/templates/core/user_edit.jinja @@ -35,7 +35,7 @@ {%- else -%} {% trans %}To edit your profile picture, ask a member of the AE{% endtrans %} {%- endif -%} - {%- if user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) and form.instance.profile_pict.id -%} + {%- if user.is_board_member and form.instance.profile_pict.id -%} {%- trans -%}Delete{%- endtrans -%} @@ -55,7 +55,7 @@

{{ form["avatar_pict"].label }}

{{ form["avatar_pict"] }} - {%- if user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) and form.instance.avatar_pict.id -%} + {%- if user.is_board_member and form.instance.avatar_pict.id -%} {%- trans -%}Delete{%- endtrans -%} @@ -75,7 +75,7 @@

{{ form["scrub_pict"].label }}

{{ form["scrub_pict"] }} - {%- if user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) and form.instance.scrub_pict.id -%} + {%- if user.is_board_member and form.instance.scrub_pict.id -%} {%- trans -%}Delete{%-endtrans -%} diff --git a/core/templates/core/user_tools.jinja b/core/templates/core/user_tools.jinja index 66e64d58..4e1e69a2 100644 --- a/core/templates/core/user_tools.jinja +++ b/core/templates/core/user_tools.jinja @@ -35,18 +35,21 @@ {% endif %} {% set is_admin_on_a_counter = false %} - {% for b in settings.SITH_COUNTER_BARS if user.is_in_group(b[1] + " admin") %} + {% for b in settings.SITH_COUNTER_BARS if user.is_in_group(name=b[1] + " admin") %} {% set is_admin_on_a_counter = true %} {% endfor %} {% if - user.is_in_group(settings.SITH_GROUP_COUNTER_ADMIN_ID) or user.is_root - or is_admin_on_a_counter + is_admin_on_a_counter + or user.is_root + or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID) %}

{% trans %}Counters{% endtrans %}

    {% for b in settings.SITH_COUNTER_BARS %} - {% if user.is_in_group(b[1]+" admin") %} + {% if user.is_in_group(name=b[1]+" admin") %} {% set c = Counter.objects.filter(id=b[0]).first() %}
  • @@ -85,13 +88,16 @@ {% endif %} {% if - user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) or user.is_root - or user.memberships.filter(end_date=None).filter(role__gte=7).all() | length > 10 + user.is_root + or user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) + or user.memberships.ongoing().filter(role__gte=7).count() > 10 %}

    {% trans %}Accounting{% endtrans %}

    {% endif %} - {% if user.is_in_group(settings.SITH_GROUP_SAS_ADMIN_ID) or user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) or user.is_root %} + {% if + user.is_root + or user.is_com_admin + or user.is_in_group(pk=settings.SITH_GROUP_SAS_ADMIN_ID) + %}

    {% trans %}Communication{% endtrans %}

    @@ -153,7 +163,10 @@
    {% endif %} - {% if user.is_in_group(settings.SITH_GROUP_PEDAGOGY_ADMIN_ID) or user.is_root %} + {% if + user.is_root + or user.is_in_group(pk=settings.SITH_GROUP_PEDAGOGY_ADMIN_ID) + %}

    {% trans %}Pedagogy{% endtrans %}

      diff --git a/core/tests.py b/core/tests.py index 7ebe697b..4c84662b 100644 --- a/core/tests.py +++ b/core/tests.py @@ -15,13 +15,18 @@ # import os +from datetime import timedelta +from django.core.cache import cache from django.test import Client, TestCase from django.urls import reverse from django.core.management import call_command +from django.utils.timezone import now -from core.models import User, Group, Page +from club.models import Membership +from core.models import User, Group, Page, AnonymousUser from core.markdown import markdown +from sith import settings """ to run these tests : @@ -30,11 +35,9 @@ to run these tests : class UserRegistrationTest(TestCase): - def setUp(self): - try: - Group.objects.create(name="root") - except Exception as e: - print(e) + @classmethod + def setUpTestData(cls): + User.objects.all().delete() def test_register_user_form_ok(self): """ @@ -282,19 +285,8 @@ class MarkdownTest(TestCase): class PageHandlingTest(TestCase): def setUp(self): - self.root_group = Group.objects.create(name="root") - u = User( - username="root", - last_name="", - first_name="Bibou", - email="ae.info@utbm.fr", - date_of_birth="1942-06-12", - is_superuser=True, - is_staff=True, - ) - u.set_password("plop") - u.save() self.client.login(username="root", password="plop") + self.root_group = Group.objects.get(name="Root") def test_create_page_ok(self): """ @@ -321,12 +313,20 @@ class PageHandlingTest(TestCase): """ Should create a page correctly """ + # remove all other pages to make sure there is no side effect + Page.objects.all().delete() self.client.post( - reverse("core:page_new"), {"parent": "", "name": "guy", "owner_group": "1"} + reverse("core:page_new"), + {"parent": "", "name": "guy", "owner_group": str(self.root_group.id)}, ) + page = Page.objects.first() response = self.client.post( reverse("core:page_new"), - {"parent": "1", "name": "bibou", "owner_group": "1"}, + { + "parent": str(page.id), + "name": "bibou", + "owner_group": str(self.root_group.id), + }, ) response = self.client.get( reverse("core:page", kwargs={"page_name": "guy/bibou"}) @@ -392,9 +392,6 @@ http://git.an class UserToolsTest(TestCase): - def setUp(self): - call_command("populate") - def test_anonymous_user_unauthorized(self): response = self.client.get(reverse("core:user_tools")) self.assertEqual(response.status_code, 403) @@ -432,13 +429,12 @@ class UserToolsTest(TestCase): class FileHandlingTest(TestCase): + @classmethod + def setUpTestData(cls): + cls.subscriber = User.objects.get(username="subscriber") + def setUp(self): - try: - call_command("populate") - self.subscriber = User.objects.filter(username="subscriber").first() - self.client.login(username="subscriber", password="plop") - except Exception as e: - print(e) + self.client.login(username="subscriber", password="plop") def test_create_folder_home(self): response = self.client.post( @@ -466,3 +462,150 @@ class FileHandlingTest(TestCase): ) self.assertTrue(response.status_code == 200) self.assertTrue("ls" in str(response.content)) + + +class UserIsInGroupTest(TestCase): + """ + Test that the User.is_in_group() and AnonymousUser.is_in_group() + work as intended + """ + + @classmethod + def setUpTestData(cls): + from club.models import Club + + cls.root_group = Group.objects.get(name="Root") + cls.public = Group.objects.get(name="Public") + cls.subscribers = Group.objects.get(name="Subscribers") + cls.old_subscribers = Group.objects.get(name="Old subscribers") + cls.accounting_admin = Group.objects.get(name="Accounting admin") + cls.com_admin = Group.objects.get(name="Communication admin") + cls.counter_admin = Group.objects.get(name="Counter admin") + cls.banned_alcohol = Group.objects.get(name="Banned from buying alcohol") + cls.banned_counters = Group.objects.get(name="Banned from counters") + cls.banned_subscription = Group.objects.get(name="Banned to subscribe") + cls.sas_admin = Group.objects.get(name="SAS admin") + cls.club = Club.objects.create( + name="Fake Club", + unix_name="fake-club", + address="Fake address", + ) + cls.main_club = Club.objects.get(id=1) + + def setUp(self) -> None: + self.toto = User.objects.create( + username="toto", first_name="a", last_name="b", email="a.b@toto.fr" + ) + self.skia = User.objects.get(username="skia") + + def assert_in_public_group(self, user): + self.assertTrue(user.is_in_group(pk=self.public.id)) + self.assertTrue(user.is_in_group(name=self.public.name)) + + def assert_in_club_metagroups(self, user, club): + meta_groups_board = club.unix_name + settings.SITH_BOARD_SUFFIX + meta_groups_members = club.unix_name + settings.SITH_MEMBER_SUFFIX + self.assertFalse(user.is_in_group(name=meta_groups_board)) + self.assertFalse(user.is_in_group(name=meta_groups_members)) + + def assert_only_in_public_group(self, user): + self.assert_in_public_group(user) + for group in ( + self.root_group, + self.banned_counters, + self.accounting_admin, + self.sas_admin, + self.subscribers, + self.old_subscribers, + ): + self.assertFalse(user.is_in_group(pk=group.pk)) + self.assertFalse(user.is_in_group(name=group.name)) + meta_groups_board = self.club.unix_name + settings.SITH_BOARD_SUFFIX + meta_groups_members = self.club.unix_name + settings.SITH_MEMBER_SUFFIX + self.assertFalse(user.is_in_group(name=meta_groups_board)) + self.assertFalse(user.is_in_group(name=meta_groups_members)) + + def test_anonymous_user(self): + """ + Test that anonymous users are only in the public group + """ + user = AnonymousUser() + self.assert_only_in_public_group(user) + + def test_not_subscribed_user(self): + """ + Test that users who never subscribed are only in the public group + """ + self.assert_only_in_public_group(self.toto) + + def test_wrong_parameter_fail(self): + """ + Test that when neither the pk nor the name argument is given, + the function raises a ValueError + """ + with self.assertRaises(ValueError): + self.toto.is_in_group() + + def test_number_queries(self): + """ + Test that the number of db queries is stable + and that less queries are made when making a new call + """ + # make sure Skia is in at least one group + self.skia.groups.add(Group.objects.first().pk) + skia_groups = self.skia.groups.all() + + group_in = skia_groups.first() + cache.clear() + # Test when the user is in the group + with self.assertNumQueries(2): + self.skia.is_in_group(pk=group_in.id) + with self.assertNumQueries(0): + self.skia.is_in_group(pk=group_in.id) + + ids = skia_groups.values_list("pk", flat=True) + group_not_in = Group.objects.exclude(pk__in=ids).first() + cache.clear() + # Test when the user is not in the group + with self.assertNumQueries(2): + self.skia.is_in_group(pk=group_not_in.id) + with self.assertNumQueries(0): + self.skia.is_in_group(pk=group_not_in.id) + + def test_cache_properly_cleared_membership(self): + """ + Test that when the membership of a user end, + the cache is properly invalidated + """ + membership = Membership.objects.create( + club=self.club, user=self.toto, end_date=None + ) + meta_groups_members = self.club.unix_name + settings.SITH_MEMBER_SUFFIX + cache.clear() + self.assertTrue(self.toto.is_in_group(name=meta_groups_members)) + self.assertEqual( + membership, cache.get(f"membership_{self.club.id}_{self.toto.id}") + ) + membership.end_date = now() - timedelta(minutes=5) + membership.save() + cached_membership = cache.get(f"membership_{self.club.id}_{self.toto.id}") + self.assertEqual(cached_membership, "not_member") + self.assertFalse(self.toto.is_in_group(name=meta_groups_members)) + + def test_cache_properly_cleared_group(self): + """ + Test that when a user is removed from a group, + the is_in_group_method return False when calling it again + """ + self.toto.groups.add(self.com_admin.pk) + self.assertTrue(self.toto.is_in_group(pk=self.com_admin.pk)) + + self.toto.groups.remove(self.com_admin.pk) + self.assertFalse(self.toto.is_in_group(pk=self.com_admin.pk)) + + def test_not_existing_group(self): + """ + Test that searching for a not existing group + returns False + """ + self.assertFalse(self.skia.is_in_group(name="This doesn't exist")) diff --git a/core/utils.py b/core/utils.py index 0e98da6b..a053e2d5 100644 --- a/core/utils.py +++ b/core/utils.py @@ -35,11 +35,13 @@ def get_git_revision_short_hash() -> str: """ Return the short hash of the current commit """ - return ( - subprocess.check_output(["git", "rev-parse", "--short", "HEAD"]) - .decode("ascii") - .strip() - ) + try: + output = subprocess.check_output(["git", "rev-parse", "--short", "HEAD"]) + if isinstance(output, bytes): + return output.decode("ascii").strip() + return output.strip() + except subprocess.CalledProcessError: + return "" def get_start_of_semester(d=date.today()): diff --git a/core/views/__init__.py b/core/views/__init__.py index 5d4d2ea5..fc807663 100644 --- a/core/views/__init__.py +++ b/core/views/__init__.py @@ -162,7 +162,6 @@ class GenericContentPermissionMixinBuilder(View): return cls.permission_function(obj, user) def dispatch(self, request, *arg, **kwargs): - if hasattr(self, "get_object") and callable(self.get_object): self.object = self.get_object() if not self.get_permission_function(self.object, request.user): diff --git a/core/views/files.py b/core/views/files.py index 2833dc7b..1047f381 100644 --- a/core/views/files.py +++ b/core/views/files.py @@ -23,7 +23,7 @@ from django.views.generic.detail import SingleObjectMixin from django.forms.models import modelform_factory from django.conf import settings from django.utils.translation import gettext_lazy as _ -from django.http import HttpResponse +from django.http import Http404, HttpResponse from wsgiref.util import FileWrapper from django.urls import reverse from django.core.exceptions import PermissionDenied @@ -34,7 +34,12 @@ import os from ajax_select import make_ajax_field from core.models import SithFile, RealGroup, Notification -from core.views import CanViewMixin, CanEditMixin, CanEditPropMixin, can_view, not_found +from core.views import ( + CanViewMixin, + CanEditMixin, + CanEditPropMixin, + can_view, +) from counter.models import Counter @@ -58,6 +63,11 @@ def send_file(request, file_id, file_class=SithFile, file_attr="file"): raise PermissionDenied name = f.__getattribute__(file_attr).name filepath = os.path.join(settings.MEDIA_ROOT, name) + + # check if file exists on disk + if not os.path.exists(filepath.encode("utf-8")): + raise Http404() + with open(filepath.encode("utf-8"), "rb") as filename: wrapper = FileWrapper(filename) response = HttpResponse(wrapper, content_type=f.mime_type) @@ -152,6 +162,13 @@ class FileEditView(CanEditMixin, UpdateView): template_name = "core/file_edit.jinja" context_object_name = "file" + def get(self, request, *args, **kwargs): + self.object = self.get_object() + if not self.object.can_be_managed_by(request.user): + raise PermissionDenied + + return super(FileEditView, self).get(request, *args, **kwargs) + def get_form_class(self): fields = ["name", "is_moderated"] if self.object.is_file: @@ -197,6 +214,13 @@ class FileEditPropView(CanEditPropMixin, UpdateView): context_object_name = "file" form_class = FileEditPropForm + def get(self, request, *args, **kwargs): + self.object = self.get_object() + if not self.object.can_be_managed_by(request.user): + raise PermissionDenied + + return super(FileEditPropView, self).get(request, *args, **kwargs) + def get_form(self, form_class=None): form = super(FileEditPropView, self).get_form(form_class) form.fields["parent"].queryset = SithFile.objects.filter(is_folder=True) @@ -269,6 +293,9 @@ class FileView(CanViewMixin, DetailView, FormMixin): def get(self, request, *args, **kwargs): self.form = self.get_form() + if not self.object.can_be_managed_by(request.user): + raise PermissionDenied + if "clipboard" not in request.session.keys(): request.session["clipboard"] = [] return super(FileView, self).get(request, *args, **kwargs) @@ -316,6 +343,13 @@ class FileDeleteView(CanEditPropMixin, DeleteView): template_name = "core/file_delete_confirm.jinja" context_object_name = "file" + def get(self, request, *args, **kwargs): + self.object = self.get_object() + if not self.object.can_be_managed_by(request.user): + raise PermissionDenied + + return super(FileDeleteView, self).get(request, *args, **kwargs) + def get_success_url(self): self.object.file.delete() # Doing it here or overloading delete() is the same, so let's do it here if "next" in self.request.GET.keys(): diff --git a/core/views/page.py b/core/views/page.py index 94356c33..c2c7dbce 100644 --- a/core/views/page.py +++ b/core/views/page.py @@ -82,6 +82,11 @@ class PageRevView(CanViewMixin, DetailView): def dispatch(self, request, *args, **kwargs): res = super(PageRevView, self).dispatch(request, *args, **kwargs) + self.object = self.get_object() + + if self.object is None: + return redirect("core:page_create", page_name=self.kwargs["page_name"]) + if self.object.need_club_redirection: return redirect( "club:club_view_rev", club_id=self.object.club.id, rev_id=kwargs["rev"] diff --git a/core/views/site.py b/core/views/site.py index 9a6355a4..c34cf2c4 100644 --- a/core/views/site.py +++ b/core/views/site.py @@ -31,6 +31,7 @@ from django.utils import html from django.views.generic import ListView, TemplateView from django.conf import settings from django.utils.text import slugify +from django.db.models.query import QuerySet import json @@ -51,12 +52,15 @@ class NotificationList(ListView): model = Notification template_name = "core/notification_list.jinja" - def get_queryset(self): + def get_queryset(self) -> QuerySet[Notification]: + if self.request.user.is_anonymous: + return Notification.objects.none() # TODO: Bulk update in django 2.2 if "see_all" in self.request.GET.keys(): for n in self.request.user.notifications.filter(viewed=False): n.viewed = True n.save() + return self.request.user.notifications.order_by("-date")[:20] diff --git a/core/views/user.py b/core/views/user.py index bcff5504..dbd60f13 100644 --- a/core/views/user.py +++ b/core/views/user.py @@ -254,10 +254,11 @@ class UserTabsMixin(TabedViewMixin): if user.customer and ( user == self.request.user or self.request.user.is_in_group( - settings.SITH_GROUP_ACCOUNTING_ADMIN_ID + pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID ) or self.request.user.is_in_group( - settings.SITH_BAR_MANAGER["unix_name"] + settings.SITH_BOARD_SUFFIX + name=settings.SITH_BAR_MANAGER["unix_name"] + + settings.SITH_BOARD_SUFFIX ) or self.request.user.is_root ): @@ -320,7 +321,7 @@ class UserPicturesView(UserTabsMixin, CanViewMixin, DetailView): last_album = None for picture in picture_qs: album = picture.parent - if album.id != last_album: + if album.id != last_album and album not in kwargs["albums"]: kwargs["albums"].append(album) kwargs["pictures"][album.id] = [] last_album = album.id @@ -413,6 +414,7 @@ class UserGodfathersTreePictureView(CanViewMixin, DetailView): self.graph = pgv.AGraph(strict=False, directed=True) family = set() self.level = 1 + # Since the tree isn't very deep, we can build it recursively def crawl_family(user): if self.level > self.depth: @@ -487,9 +489,9 @@ class UserStatsView(UserTabsMixin, CanViewMixin, DetailView): if not ( profile == request.user - or request.user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) + or request.user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) or request.user.is_in_group( - settings.SITH_BAR_MANAGER["unix_name"] + settings.SITH_BOARD_SUFFIX + name=settings.SITH_BAR_MANAGER["unix_name"] + settings.SITH_BOARD_SUFFIX ) or request.user.is_root ): @@ -717,8 +719,12 @@ class UserPreferencesView(UserTabsMixin, CanEditMixin, UpdateView): def get_context_data(self, **kwargs): kwargs = super(UserPreferencesView, self).get_context_data(**kwargs) - if not hasattr(self.object, "trombi_user"): + + if not ( + hasattr(self.object, "trombi_user") and self.request.user.trombi_user.trombi + ): kwargs["trombi_form"] = UserTrombiForm() + if hasattr(self.object, "customer"): kwargs["student_card_form"] = StudentCardForm() return kwargs @@ -771,9 +777,9 @@ class UserAccountBase(UserTabsMixin, DetailView): res = super(UserAccountBase, self).dispatch(request, *arg, **kwargs) if ( self.object == request.user - or request.user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) + or request.user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) or request.user.is_in_group( - settings.SITH_BAR_MANAGER["unix_name"] + settings.SITH_BOARD_SUFFIX + name=settings.SITH_BAR_MANAGER["unix_name"] + settings.SITH_BOARD_SUFFIX ) or request.user.is_root ): diff --git a/counter/migrations/0001_initial.py b/counter/migrations/0001_initial.py index de31a6e2..e9635e5d 100644 --- a/counter/migrations/0001_initial.py +++ b/counter/migrations/0001_initial.py @@ -9,7 +9,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [ ("subscription", "0001_initial"), migrations.swappable_dependency(settings.AUTH_USER_MODEL), diff --git a/counter/migrations/0002_auto_20160826_1342.py b/counter/migrations/0002_auto_20160826_1342.py index 9b97700e..ccc4288c 100644 --- a/counter/migrations/0002_auto_20160826_1342.py +++ b/counter/migrations/0002_auto_20160826_1342.py @@ -8,7 +8,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [ migrations.swappable_dependency(settings.AUTH_USER_MODEL), ("counter", "0001_initial"), diff --git a/counter/migrations/0003_permanency_activity.py b/counter/migrations/0003_permanency_activity.py index 05707b20..be4e9962 100644 --- a/counter/migrations/0003_permanency_activity.py +++ b/counter/migrations/0003_permanency_activity.py @@ -7,7 +7,6 @@ from django.utils.timezone import utc class Migration(migrations.Migration): - dependencies = [("counter", "0002_auto_20160826_1342")] operations = [ diff --git a/counter/migrations/0004_auto_20160826_1907.py b/counter/migrations/0004_auto_20160826_1907.py index 6e2ddd21..ce4d6180 100644 --- a/counter/migrations/0004_auto_20160826_1907.py +++ b/counter/migrations/0004_auto_20160826_1907.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("counter", "0003_permanency_activity")] operations = [ diff --git a/counter/migrations/0005_auto_20160826_2330.py b/counter/migrations/0005_auto_20160826_2330.py index 37f3510e..8e0df744 100644 --- a/counter/migrations/0005_auto_20160826_2330.py +++ b/counter/migrations/0005_auto_20160826_2330.py @@ -6,7 +6,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [("counter", "0004_auto_20160826_1907")] operations = [ diff --git a/counter/migrations/0006_auto_20160831_1304.py b/counter/migrations/0006_auto_20160831_1304.py index 15b7b169..9ae1f4ec 100644 --- a/counter/migrations/0006_auto_20160831_1304.py +++ b/counter/migrations/0006_auto_20160831_1304.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("counter", "0005_auto_20160826_2330")] operations = [ diff --git a/counter/migrations/0007_product_archived.py b/counter/migrations/0007_product_archived.py index 521eda48..5dd3140d 100644 --- a/counter/migrations/0007_product_archived.py +++ b/counter/migrations/0007_product_archived.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("counter", "0006_auto_20160831_1304")] operations = [ diff --git a/counter/migrations/0008_counter_token.py b/counter/migrations/0008_counter_token.py index a02b5a5d..f8a5595e 100644 --- a/counter/migrations/0008_counter_token.py +++ b/counter/migrations/0008_counter_token.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("counter", "0007_product_archived")] operations = [ diff --git a/counter/migrations/0009_eticket.py b/counter/migrations/0009_eticket.py index 23b9204c..68b675ef 100644 --- a/counter/migrations/0009_eticket.py +++ b/counter/migrations/0009_eticket.py @@ -6,7 +6,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [("counter", "0008_counter_token")] operations = [ diff --git a/counter/migrations/0010_auto_20161003_1900.py b/counter/migrations/0010_auto_20161003_1900.py index e438b05e..91ed0756 100644 --- a/counter/migrations/0010_auto_20161003_1900.py +++ b/counter/migrations/0010_auto_20161003_1900.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("counter", "0009_eticket")] operations = [ diff --git a/counter/migrations/0011_auto_20161004_2039.py b/counter/migrations/0011_auto_20161004_2039.py index c70bad63..439cd354 100644 --- a/counter/migrations/0011_auto_20161004_2039.py +++ b/counter/migrations/0011_auto_20161004_2039.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("counter", "0010_auto_20161003_1900")] operations = [ diff --git a/counter/migrations/0012_auto_20170515_2202.py b/counter/migrations/0012_auto_20170515_2202.py index e50d4856..2c11c82f 100644 --- a/counter/migrations/0012_auto_20170515_2202.py +++ b/counter/migrations/0012_auto_20170515_2202.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("counter", "0011_auto_20161004_2039")] operations = [ diff --git a/counter/migrations/0013_customer_recorded_products.py b/counter/migrations/0013_customer_recorded_products.py index 80f6d94d..d6b3cfa8 100644 --- a/counter/migrations/0013_customer_recorded_products.py +++ b/counter/migrations/0013_customer_recorded_products.py @@ -37,7 +37,6 @@ def balance_ecocups(apps, schema_editor): class Migration(migrations.Migration): - dependencies = [("counter", "0012_auto_20170515_2202")] operations = [ diff --git a/counter/migrations/0014_auto_20170816_1521.py b/counter/migrations/0014_auto_20170816_1521.py index 7e13c170..14c64d8d 100644 --- a/counter/migrations/0014_auto_20170816_1521.py +++ b/counter/migrations/0014_auto_20170816_1521.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("counter", "0013_customer_recorded_products")] operations = [ diff --git a/counter/migrations/0014_auto_20170817_1537.py b/counter/migrations/0014_auto_20170817_1537.py index 67acbc13..8fef9833 100644 --- a/counter/migrations/0014_auto_20170817_1537.py +++ b/counter/migrations/0014_auto_20170817_1537.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("counter", "0013_customer_recorded_products")] operations = [ diff --git a/counter/migrations/0015_merge.py b/counter/migrations/0015_merge.py index aefcfaca..6dcff021 100644 --- a/counter/migrations/0015_merge.py +++ b/counter/migrations/0015_merge.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [ ("counter", "0014_auto_20170817_1537"), ("counter", "0014_auto_20170816_1521"), diff --git a/counter/migrations/0016_producttype_comment.py b/counter/migrations/0016_producttype_comment.py index 295553b2..ad5ad487 100644 --- a/counter/migrations/0016_producttype_comment.py +++ b/counter/migrations/0016_producttype_comment.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("counter", "0015_merge")] operations = [ diff --git a/counter/migrations/0017_studentcard.py b/counter/migrations/0017_studentcard.py index 8304a104..a2f62222 100644 --- a/counter/migrations/0017_studentcard.py +++ b/counter/migrations/0017_studentcard.py @@ -8,7 +8,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [("counter", "0016_producttype_comment")] operations = [ diff --git a/counter/migrations/0018_producttype_priority.py b/counter/migrations/0018_producttype_priority.py index 89149507..6bee9277 100644 --- a/counter/migrations/0018_producttype_priority.py +++ b/counter/migrations/0018_producttype_priority.py @@ -4,7 +4,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [ ("counter", "0017_studentcard"), ] diff --git a/counter/migrations/0019_billinginfo.py b/counter/migrations/0019_billinginfo.py index 4a8af24b..c4c74c39 100644 --- a/counter/migrations/0019_billinginfo.py +++ b/counter/migrations/0019_billinginfo.py @@ -6,7 +6,6 @@ import django_countries.fields class Migration(migrations.Migration): - dependencies = [ ("counter", "0018_producttype_priority"), ] diff --git a/counter/migrations/0020_auto_20221215_1709.py b/counter/migrations/0020_auto_20221215_1709.py index ca5d1176..52010133 100644 --- a/counter/migrations/0020_auto_20221215_1709.py +++ b/counter/migrations/0020_auto_20221215_1709.py @@ -5,7 +5,6 @@ from django.db import migrations class Migration(migrations.Migration): - dependencies = [ ("counter", "0019_billinginfo"), ] diff --git a/counter/models.py b/counter/models.py index f0e96099..c6389349 100644 --- a/counter/models.py +++ b/counter/models.py @@ -228,7 +228,9 @@ class ProductType(models.Model): """ Method to see if that object can be edited by the given user """ - if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): + if user.is_anonymous: + return False + if user.is_in_group(pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID): return True return False @@ -294,9 +296,11 @@ class Product(models.Model): """ Method to see if that object can be edited by the given user """ + if user.is_anonymous: + return False if user.is_in_group( - settings.SITH_GROUP_ACCOUNTING_ADMIN_ID - ) or user.is_in_group(settings.SITH_GROUP_COUNTER_ADMIN_ID): + pk=settings.SITH_GROUP_ACCOUNTING_ADMIN_ID + ) or user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID): return True return False @@ -318,8 +322,8 @@ class Product(models.Model): """ if not self.buying_groups.exists(): return True - for group in self.buying_groups.all(): - if user.is_in_group(group.name): + for group_id in self.buying_groups.values_list("pk", flat=True): + if user.is_in_group(pk=group_id): return True return False @@ -402,18 +406,17 @@ class Counter(models.Model): return reverse("counter:details", kwargs={"counter_id": self.id}) def is_owned_by(self, user): + if user.is_anonymous: + return False mem = self.club.get_membership_for(user) if mem and mem.role >= 7: return True - return user.is_in_group(settings.SITH_GROUP_COUNTER_ADMIN_ID) + return user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID) def can_be_viewed_by(self, user): if self.type == "BAR": return True - return ( - user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) - or user in self.sellers.all() - ) + return user.is_board_member or user in self.sellers.all() def gen_token(self): """Generate a new token for this counter""" @@ -621,6 +624,8 @@ class Refilling(models.Model): ) def is_owned_by(self, user): + if user.is_anonymous: + return False return user.is_owner(self.counter) and self.payment_method != "CARD" def delete(self, *args, **kwargs): @@ -713,6 +718,8 @@ class Selling(models.Model): ) def is_owned_by(self, user): + if user.is_anonymous: + return False return user.is_owner(self.counter) and self.payment_method != "CARD" def can_be_viewed_by(self, user): @@ -953,7 +960,9 @@ class CashRegisterSummary(models.Model): """ Method to see if that object can be edited by the given user """ - if user.is_in_group(settings.SITH_GROUP_COUNTER_ADMIN_ID): + if user.is_anonymous: + return False + if user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID): return True return False @@ -1022,7 +1031,9 @@ class Eticket(models.Model): """ Method to see if that object can be edited by the given user """ - return user.is_in_group(settings.SITH_GROUP_COUNTER_ADMIN_ID) + if user.is_anonymous: + return False + return user.is_in_group(pk=settings.SITH_GROUP_COUNTER_ADMIN_ID) def get_hash(self, string): import hashlib diff --git a/counter/tests.py b/counter/tests.py index 13c4403b..ed83e935 100644 --- a/counter/tests.py +++ b/counter/tests.py @@ -30,13 +30,13 @@ from sith.settings import SITH_MAIN_CLUB class CounterTest(TestCase): - def setUp(self): - call_command("populate") - self.skia = User.objects.filter(username="skia").first() - self.sli = User.objects.filter(username="sli").first() - self.krophil = User.objects.filter(username="krophil").first() - self.mde = Counter.objects.filter(name="MDE").first() - self.foyer = Counter.objects.get(id=2) + @classmethod + def setUpTestData(cls): + cls.skia = User.objects.filter(username="skia").first() + cls.sli = User.objects.filter(username="sli").first() + cls.krophil = User.objects.filter(username="krophil").first() + cls.mde = Counter.objects.filter(name="MDE").first() + cls.foyer = Counter.objects.get(id=2) def test_full_click(self): self.client.post( @@ -161,9 +161,7 @@ class CounterTest(TestCase): class CounterStatsTest(TestCase): @classmethod - def setUpClass(cls): - super().setUpClass() - call_command("populate") + def setUpTestData(cls): cls.counter = Counter.objects.filter(id=2).first() cls.krophil = User.objects.get(username="krophil") cls.skia = User.objects.get(username="skia") @@ -171,10 +169,7 @@ class CounterStatsTest(TestCase): cls.root = User.objects.get(username="root") cls.subscriber = User.objects.get(username="subscriber") cls.old_subscriber = User.objects.get(username="old_subscriber") - cls.counter.sellers.add(cls.sli) - cls.counter.sellers.add(cls.root) - cls.counter.sellers.add(cls.skia) - cls.counter.sellers.add(cls.krophil) + cls.counter.sellers.add(cls.sli, cls.root, cls.skia, cls.krophil) barbar = Product.objects.get(code="BARB") @@ -368,7 +363,7 @@ class CounterStatsTest(TestCase): class BillingInfoTest(TestCase): @classmethod - def setUpClass(cls): + def setUpTestData(cls): cls.payload_1 = { "first_name": "Subscribed", "last_name": "User", @@ -385,8 +380,6 @@ class BillingInfoTest(TestCase): "city": "Sète", "country": "FR", } - super().setUpClass() - call_command("populate") def test_edit_infos(self): user = User.objects.get(username="subscriber") @@ -596,7 +589,6 @@ class BillingInfoTest(TestCase): class BarmanConnectionTest(TestCase): def setUp(self): - call_command("populate") self.krophil = User.objects.get(username="krophil") self.skia = User.objects.get(username="skia") self.skia.customer.account = 800 @@ -662,7 +654,6 @@ class StudentCardTest(TestCase): """ def setUp(self): - call_command("populate") self.krophil = User.objects.get(username="krophil") self.sli = User.objects.get(username="sli") diff --git a/counter/views.py b/counter/views.py index e0e5b658..4d5af292 100644 --- a/counter/views.py +++ b/counter/views.py @@ -87,8 +87,8 @@ class CounterAdminMixin(View): edit_club = [] def _test_group(self, user): - for g in self.edit_group: - if user.is_in_group(g): + for grp_id in self.edit_group: + if user.is_in_group(pk=grp_id): return True return False @@ -486,14 +486,12 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): pid = str(pid) price = self.get_price(pid) total = self.sum_basket(request) - product = self.get_product(pid) - can_buy = False - if not product.buying_groups.exists(): - can_buy = True - else: - for g in product.buying_groups.all(): - if self.customer.user.is_in_group(g.name): - can_buy = True + product: Product = self.get_product(pid) + user: User = self.customer.user + buying_groups = list(product.buying_groups.values_list("pk", flat=True)) + can_buy = len(buying_groups) == 0 or any( + user.is_in_group(pk=group_id) for group_id in buying_groups + ) if not can_buy: request.session["not_allowed"] = True return False @@ -514,18 +512,17 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): ): request.session["not_allowed"] = True return False - if product.limit_age >= 18 and not self.customer.user.date_of_birth: + if product.limit_age >= 18 and not user.date_of_birth: request.session["no_age"] = True return False - if product.limit_age >= 18 and self.customer.user.is_banned_alcohol: + if product.limit_age >= 18 and user.is_banned_alcohol: request.session["not_allowed"] = True return False - if self.customer.user.is_banned_counter: + if user.is_banned_counter: request.session["not_allowed"] = True return False if ( - self.customer.user.date_of_birth - and self.customer.user.get_age() < product.limit_age + user.date_of_birth and self.customer.user.get_age() < product.limit_age ): # Check if affordable request.session["too_young"] = True return False @@ -586,7 +583,7 @@ class CounterClick(CounterTabsMixin, CanViewMixin, DetailView): - , where the string is the code of the product - X, where the integer is the quantity and str the code """ - string = parse_qs(request.body.decode())["code"][0].upper() + string = parse_qs(request.body.decode()).get("code", [""])[0].upper() if string == "FIN": return self.finish(request) elif string == "ANN": diff --git a/doc/TW_Skia/Rapport.tex b/doc/TW_Skia/Rapport.tex index 7295fbb8..49b84bfa 100644 --- a/doc/TW_Skia/Rapport.tex +++ b/doc/TW_Skia/Rapport.tex @@ -342,7 +342,7 @@ tous les autres. {% if user.is_in_group(settings.SITH_GROUP_ACCOUNTING_ADMIN_ID) %}
    • Accounting
    • {% endif %} -{% if user.is_in_group(settings.SITH_MAIN_BOARD_GROUP) or user.is_root %} +{% if user.is_board_member or user.is_root %}
    • Subscriptions
    • Counters management
    • {% endif %} diff --git a/eboutic/migrations/0001_initial.py b/eboutic/migrations/0001_initial.py index 8dd12da2..b642642f 100644 --- a/eboutic/migrations/0001_initial.py +++ b/eboutic/migrations/0001_initial.py @@ -8,7 +8,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [migrations.swappable_dependency(settings.AUTH_USER_MODEL)] operations = [ diff --git a/eboutic/migrations/0002_auto_20221005_2243.py b/eboutic/migrations/0002_auto_20221005_2243.py index 2c1138ae..2ed1c8d0 100644 --- a/eboutic/migrations/0002_auto_20221005_2243.py +++ b/eboutic/migrations/0002_auto_20221005_2243.py @@ -4,7 +4,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [ ("eboutic", "0001_initial"), ] diff --git a/eboutic/tests.py b/eboutic/tests.py index b5d4d6ce..af0e5850 100644 --- a/eboutic/tests.py +++ b/eboutic/tests.py @@ -41,7 +41,6 @@ from eboutic.models import Basket class EbouticTest(TestCase): @classmethod def setUpTestData(cls): - call_command("populate") cls.barbar = Product.objects.filter(code="BARB").first() cls.refill = Product.objects.filter(code="15REFILL").first() cls.cotis = Product.objects.filter(code="1SCOTIZ").first() diff --git a/eboutic/views.py b/eboutic/views.py index a54ed793..885edf43 100644 --- a/eboutic/views.py +++ b/eboutic/views.py @@ -174,8 +174,8 @@ def pay_with_sith(request): class EtransactionAutoAnswer(View): - # Response documentation http://www1.paybox.com/espace-integrateur-documentation - # /la-solution-paybox-system/gestion-de-la-reponse/ + # Response documentation + # https://www1.paybox.com/espace-integrateur-documentation/la-solution-paybox-system/gestion-de-la-reponse/ def get(self, request, *args, **kwargs): required = {"Amount", "BasketID", "Error", "Sig"} if not required.issubset(set(request.GET.keys())): diff --git a/election/migrations/0001_initial.py b/election/migrations/0001_initial.py index 943928cc..d3c4c449 100644 --- a/election/migrations/0001_initial.py +++ b/election/migrations/0001_initial.py @@ -7,7 +7,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [ ("core", "0018_auto_20161224_0211"), migrations.swappable_dependency(settings.AUTH_USER_MODEL), diff --git a/election/migrations/0002_election_archived.py b/election/migrations/0002_election_archived.py index c113cf46..2b5ed86c 100644 --- a/election/migrations/0002_election_archived.py +++ b/election/migrations/0002_election_archived.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("election", "0001_initial")] operations = [ diff --git a/election/migrations/0003_auto_20171202_1819.py b/election/migrations/0003_auto_20171202_1819.py index e918bc06..6cb48dc6 100644 --- a/election/migrations/0003_auto_20171202_1819.py +++ b/election/migrations/0003_auto_20171202_1819.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("election", "0002_election_archived")] operations = [ diff --git a/election/migrations/0004_auto_20191006_0049.py b/election/migrations/0004_auto_20191006_0049.py index 01f5c1f9..7e553483 100644 --- a/election/migrations/0004_auto_20191006_0049.py +++ b/election/migrations/0004_auto_20191006_0049.py @@ -4,7 +4,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("election", "0003_auto_20171202_1819")] operations = [ diff --git a/election/models.py b/election/models.py index 537cc6e9..f100a1d6 100644 --- a/election/models.py +++ b/election/models.py @@ -73,16 +73,16 @@ class Election(models.Model): return bool(timezone.now() <= self.end_candidature) def can_candidate(self, user): - for group in self.candidature_groups.all(): - if user.is_in_group(group): + for group_id in self.candidature_groups.values_list("pk", flat=True): + if user.is_in_group(pk=group_id): return True return False def can_vote(self, user): if not self.is_vote_active or self.has_voted(user): return False - for group in self.vote_groups.all(): - if user.is_in_group(group): + for group_id in self.vote_groups.values_list("pk", flat=True): + if user.is_in_group(pk=group_id): return True return False @@ -97,10 +97,9 @@ class Election(models.Model): results[role.title] = role.results(total_vote) return results - def delete(self): - for election_list in self.election_lists.all(): - election_list.delete() - super(Election, self).delete() + def delete(self, *args, **kwargs): + self.election_lists.all().delete() + super(Election, self).delete(*args, **kwargs) # Permissions diff --git a/election/tests.py b/election/tests.py index 31934187..03b46481 100644 --- a/election/tests.py +++ b/election/tests.py @@ -9,8 +9,6 @@ from election.models import Election class MainElection(TestCase): def setUp(self): - call_command("populate") - self.election = Election.objects.all().first() self.public_group = Group.objects.get(id=settings.SITH_GROUP_PUBLIC_ID) self.subscriber_group = Group.objects.get(name=settings.SITH_MAIN_MEMBERS_GROUP) diff --git a/election/views.py b/election/views.py index f36482fa..a223c78c 100644 --- a/election/views.py +++ b/election/views.py @@ -2,14 +2,14 @@ from django.shortcuts import get_object_or_404 from django.views.generic import ListView, DetailView from django.views.generic.edit import UpdateView, CreateView from django.views.generic.edit import DeleteView, FormView -from django.urls import reverse_lazy +from django.urls import reverse_lazy, reverse from django.utils.translation import gettext_lazy as _ from django.core.exceptions import PermissionDenied from django.db import transaction -from django.forms import CheckboxSelectMultiple from django.shortcuts import redirect from django import forms +from core.models import User from core.views import CanViewMixin, CanEditMixin, CanCreateMixin from django.db.models.query import QuerySet from core.views.forms import SelectDateTime, MarkdownInput @@ -224,8 +224,8 @@ class ElectionDetailView(CanViewMixin, DetailView): pk_url_kwarg = "election_id" def get(self, request, *arg, **kwargs): - r = super(ElectionDetailView, self).get(request, *arg, **kwargs) - election = self.get_object() + response = super(ElectionDetailView, self).get(request, *arg, **kwargs) + election: Election = self.get_object() if request.user.can_edit(election) and election.is_vote_editable: action = request.GET.get("action", None) role = request.GET.get("role", None) @@ -239,9 +239,9 @@ class ElectionDetailView(CanViewMixin, DetailView): elif action == "top": Role.objects.get(id=role).top() return redirect( - reverse_lazy("election:detail", kwargs={"election_id": election.id}) + reverse("election:detail", kwargs={"election_id": election.id}) ) - return r + return response def get_context_data(self, **kwargs): """Add additionnal data to the template""" @@ -295,8 +295,8 @@ class VoteFormView(CanCreateMixin, FormView): """ data = form.clean() res = super(FormView, self).form_valid(form) - for grp in self.election.vote_groups.all(): - if self.request.user.is_in_group(grp): + for grp_id in self.election.vote_groups.values_list("pk", flat=True): + if self.request.user.is_in_group(pk=grp_id): self.vote(data) return res return res @@ -401,12 +401,13 @@ class RoleCreateView(CanCreateMixin, CreateView): def form_valid(self, form): """ - Verify that the user can edit proprely + Verify that the user can edit properly """ - obj = form.instance + obj: Role = form.instance + user: User = self.request.user if obj.election: - for grp in obj.election.edit_groups.all(): - if self.request.user.is_in_group(grp): + for grp_id in obj.election.edit_groups.values_list("pk", flat=True): + if user.is_in_group(pk=grp_id): return super(CreateView, self).form_valid(form) raise PermissionDenied @@ -446,13 +447,14 @@ class ElectionListCreateView(CanCreateMixin, CreateView): """ Verify that the user can vote on this election """ - obj = form.instance + obj: ElectionList = form.instance + user: User = self.request.user if obj.election: - for grp in obj.election.candidature_groups.all(): - if self.request.user.is_in_group(grp): + for grp_id in obj.election.candidature_groups.values_list("pk", flat=True): + if user.is_in_group(pk=grp_id): return super(CreateView, self).form_valid(form) - for grp in obj.election.edit_groups.all(): - if self.request.user.is_in_group(grp): + for grp_id in obj.election.edit_groups.values_list("pk", flat=True): + if user.is_in_group(pk=grp_id): return super(CreateView, self).form_valid(form) raise PermissionDenied diff --git a/forum/migrations/0001_initial.py b/forum/migrations/0001_initial.py index 866c25e8..6c06b0a6 100644 --- a/forum/migrations/0001_initial.py +++ b/forum/migrations/0001_initial.py @@ -10,7 +10,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [ migrations.swappable_dependency(settings.AUTH_USER_MODEL), ("club", "0006_auto_20161229_0040"), diff --git a/forum/migrations/0002_auto_20170312_1753.py b/forum/migrations/0002_auto_20170312_1753.py index a0807c47..2634d59e 100644 --- a/forum/migrations/0002_auto_20170312_1753.py +++ b/forum/migrations/0002_auto_20170312_1753.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("forum", "0001_initial")] operations = [ diff --git a/forum/migrations/0003_auto_20170510_1754.py b/forum/migrations/0003_auto_20170510_1754.py index db175969..b708ae06 100644 --- a/forum/migrations/0003_auto_20170510_1754.py +++ b/forum/migrations/0003_auto_20170510_1754.py @@ -5,7 +5,6 @@ from django.db import migrations, models class Migration(migrations.Migration): - dependencies = [("forum", "0002_auto_20170312_1753")] operations = [ diff --git a/forum/migrations/0004_auto_20170531_1949.py b/forum/migrations/0004_auto_20170531_1949.py index d6c4a660..152bdaea 100644 --- a/forum/migrations/0004_auto_20170531_1949.py +++ b/forum/migrations/0004_auto_20170531_1949.py @@ -6,7 +6,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [("forum", "0003_auto_20170510_1754")] operations = [ diff --git a/forum/migrations/0005_forumtopic_subscribed_users.py b/forum/migrations/0005_forumtopic_subscribed_users.py index 9e6e83c7..b1f6d629 100644 --- a/forum/migrations/0005_forumtopic_subscribed_users.py +++ b/forum/migrations/0005_forumtopic_subscribed_users.py @@ -6,7 +6,6 @@ from django.conf import settings class Migration(migrations.Migration): - dependencies = [ migrations.swappable_dependency(settings.AUTH_USER_MODEL), ("forum", "0004_auto_20170531_1949"), diff --git a/forum/migrations/0006_auto_20180426_2013.py b/forum/migrations/0006_auto_20180426_2013.py index 08ceed56..30248dec 100644 --- a/forum/migrations/0006_auto_20180426_2013.py +++ b/forum/migrations/0006_auto_20180426_2013.py @@ -6,7 +6,6 @@ import forum.models class Migration(migrations.Migration): - dependencies = [("forum", "0005_forumtopic_subscribed_users")] operations = [ diff --git a/forum/models.py b/forum/models.py index dd8722e9..3894caa7 100644 --- a/forum/models.py +++ b/forum/models.py @@ -157,10 +157,13 @@ class Forum(models.Model): self.save() _club_memberships = {} # This cache is particularly efficient: + # divided by 3 the number of requests on the main forum page # after the first load def is_owned_by(self, user): - if user.is_in_group(settings.SITH_GROUP_FORUM_ADMIN_ID): + if user.is_anonymous: + return False + if user.is_in_group(pk=settings.SITH_GROUP_FORUM_ADMIN_ID): return True try: m = Forum._club_memberships[self.id][user.id] @@ -336,7 +339,10 @@ class ForumMessage(models.Model): def is_last_in_topic(self): return bool(self.id == self.topic.messages.order_by("date").last().id) - def is_owned_by(self, user): # Anyone can create a topic: it's better to + def is_owned_by(self, user): + if user.is_anonymous: + return False + # Anyone can create a topic: it's better to # check the rights at the forum level, since it's more controlled return self.topic.forum.is_owned_by(user) or user.id == self.author.id diff --git a/forum/templates/forum/forum.jinja b/forum/templates/forum/forum.jinja index 1242e50d..57031b19 100644 --- a/forum/templates/forum/forum.jinja +++ b/forum/templates/forum/forum.jinja @@ -10,7 +10,11 @@

      {{ forum.name }}

      - {% if user.is_in_group(settings.SITH_GROUP_FORUM_ADMIN_ID) or user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) or user.can_edit(forum) %} + {% + if user.is_com_admin + or user.is_in_group(pk=settings.SITH_GROUP_FORUM_ADMIN_ID) + or user.can_edit(forum) + %} {% trans %}New forum{% endtrans %}
      {% endif %} {% if not forum.is_category %} diff --git a/forum/templates/forum/main.jinja b/forum/templates/forum/main.jinja index 8bedc310..e6a62785 100644 --- a/forum/templates/forum/main.jinja +++ b/forum/templates/forum/main.jinja @@ -17,7 +17,10 @@ {% trans %}Favorite topics{% endtrans %} {{ display_search_bar(request) }}

      - {% if user.is_in_group(settings.SITH_GROUP_FORUM_ADMIN_ID) or user.is_in_group(settings.SITH_GROUP_COM_ADMIN_ID) %} + {% if + user.is_com_admin + or user.is_in_group(pk=settings.SITH_GROUP_FORUM_ADMIN_ID) + %}

      {% trans %}New forum{% endtrans %}

      diff --git a/galaxy/migrations/0001_initial.py b/galaxy/migrations/0001_initial.py index e155d1cb..ab9a6a06 100644 --- a/galaxy/migrations/0001_initial.py +++ b/galaxy/migrations/0001_initial.py @@ -6,7 +6,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - initial = True dependencies = [ diff --git a/galaxy/tests.py b/galaxy/tests.py index d5957a16..c30ec8cf 100644 --- a/galaxy/tests.py +++ b/galaxy/tests.py @@ -31,7 +31,6 @@ from galaxy.models import Galaxy class GalaxyTest(TestCase): def setUp(self): - call_command("populate") self.root = User.objects.get(username="root") self.skia = User.objects.get(username="skia") self.sli = User.objects.get(username="sli") diff --git a/launderette/migrations/0001_initial.py b/launderette/migrations/0001_initial.py index c6746b2c..79f011a7 100644 --- a/launderette/migrations/0001_initial.py +++ b/launderette/migrations/0001_initial.py @@ -6,7 +6,6 @@ import django.db.models.deletion class Migration(migrations.Migration): - dependencies = [("subscription", "0001_initial"), ("counter", "0001_initial")] operations = [ diff --git a/launderette/models.py b/launderette/models.py index 7d8b2267..3ca12b2d 100644 --- a/launderette/models.py +++ b/launderette/models.py @@ -42,6 +42,8 @@ class Launderette(models.Model): """ Method to see if that object can be edited by the given user """ + if user.is_anonymous: + return False launderette_club = Club.objects.filter( unix_name=settings.SITH_LAUNDERETTE_MANAGER["unix_name"] ).first() @@ -60,7 +62,7 @@ class Launderette(models.Model): return False def can_be_viewed_by(self, user): - return user.is_in_group(settings.SITH_MAIN_MEMBERS_GROUP) + return user.is_subscribed def __str__(self): return self.name @@ -101,6 +103,8 @@ class Machine(models.Model): """ Method to see if that object can be edited by the given user """ + if user.is_anonymous: + return False launderette_club = Club.objects.filter( unix_name=settings.SITH_LAUNDERETTE_MANAGER["unix_name"] ).first() @@ -155,6 +159,8 @@ class Token(models.Model): """ Method to see if that object can be edited by the given user """ + if user.is_anonymous: + return False launderette_club = Club.objects.filter( unix_name=settings.SITH_LAUNDERETTE_MANAGER["unix_name"] ).first() diff --git a/launderette/templates/launderette/launderette_book_choose.jinja b/launderette/templates/launderette/launderette_book_choose.jinja index af30c6c1..d80505b4 100644 --- a/launderette/templates/launderette/launderette_book_choose.jinja +++ b/launderette/templates/launderette/launderette_book_choose.jinja @@ -5,7 +5,7 @@ {% endblock %} {% block content %} -{% if request.user.is_in_group(settings.SITH_MAIN_MEMBERS_GROUP) %} +{% if request.user.is_subscribed %}
      diff --git a/trombi/views.py b/trombi/views.py index e646fdd6..98bf5fc2 100644 --- a/trombi/views.py +++ b/trombi/views.py @@ -100,7 +100,6 @@ class UserIsInATrombiMixin(View): """ def dispatch(self, request, *args, **kwargs): - if not hasattr(self.request.user, "trombi_user"): raise Http404() @@ -463,6 +462,10 @@ class UserTrombiProfileView(TrombiTabsMixin, DetailView): def get(self, request, *args, **kwargs): self.object = self.get_object() + + if request.user.is_anonymous: + raise PermissionDenied() + if ( self.object.trombi.id != request.user.trombi_user.trombi.id or self.object.user.id == request.user.id