Skip to content

Commit f5a81b7

Browse files
Anas12091101Anas12091101
committed
fix: issues
1 parent 1a2cb4a commit f5a81b7

12 files changed

+290
-223
lines changed

‎ecommerce/views.py

+24-33
Original file line numberDiff line numberDiff line change
@@ -384,43 +384,34 @@ def ecommerce_restricted(request):
384384
"""
385385
Views accessible only to users with permissions to create or modify coupons.
386386
"""
387-
has_coupon_add_permission = request.user.has_perm(COUPON_ADD_PERMISSION)
388-
has_coupon_update_permission = request.user.has_perm(COUPON_UPDATE_PERMISSION)
389-
has_coupon_product_assignment_permission = request.user.has_perm(
390-
COUPON_PRODUCT_ASSIGNMENT_ADD_PERMISSION
391-
) and request.user.has_perm(COUPON_PRODUCT_ASSIGNMENT_UPDATE_PERMISSION)
392-
393-
if not (
394-
has_coupon_add_permission
395-
or has_coupon_update_permission
396-
or has_coupon_product_assignment_permission
397-
):
398-
raise PermissionDenied
387+
user = request.user
388+
permissions = {
389+
"has_coupon_create_permission": user.has_perm(COUPON_ADD_PERMISSION),
390+
"has_coupon_update_permission": user.has_perm(COUPON_UPDATE_PERMISSION),
391+
"has_coupon_product_assignment_permission": (
392+
user.has_perm(COUPON_PRODUCT_ASSIGNMENT_ADD_PERMISSION)
393+
and user.has_perm(COUPON_PRODUCT_ASSIGNMENT_UPDATE_PERMISSION)
394+
),
395+
}
399396

400-
if (
401-
(
402-
request.path.startswith("/ecommerce/admin/coupons")
403-
and not has_coupon_add_permission
404-
)
405-
or (
406-
request.path.startswith("/ecommerce/admin/deactivate-coupons")
407-
and not has_coupon_update_permission
408-
)
409-
or (
410-
request.path.startswith("/ecommerce/admin/process-coupon-assignment-sheets")
411-
and not has_coupon_product_assignment_permission
412-
)
413-
):
397+
# Deny access if the user lacks all relevant permissions
398+
if not any(permissions.values()):
414399
raise PermissionDenied
415400

401+
# Define path-based permission checks
402+
restricted_paths = {
403+
"/ecommerce/admin/coupons": "has_coupon_create_permission",
404+
"/ecommerce/admin/deactivate-coupons": "has_coupon_update_permission",
405+
"/ecommerce/admin/process-coupon-assignment-sheets": "has_coupon_product_assignment_permission",
406+
}
407+
408+
for path, perm in restricted_paths.items():
409+
if request.path.startswith(path) and not permissions[perm]:
410+
raise PermissionDenied
411+
416412
context = get_base_context(request)
417-
context["user_permissions"] = json.dumps(
418-
{
419-
"has_coupon_create_permission": has_coupon_add_permission,
420-
"has_coupon_update_permission": has_coupon_update_permission,
421-
"has_coupon_product_assignment_permission": has_coupon_product_assignment_permission,
422-
}
423-
)
413+
context["user_permissions"] = json.dumps(permissions)
414+
424415
return render(request, "index.html", context=context)
425416

426417

‎sheets/management/commands/process_coupon_assignment_sheet.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
from django.core.management import BaseCommand, CommandError
77

8-
from sheets.management.utils import assign_coupons_from_spreadsheet
8+
from sheets.utils import assign_coupons_from_spreadsheet
99
from sheets.exceptions import CouponAssignmentError
1010

1111

‎sheets/management/utils.py

-60
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@
22

33
from sheets.coupon_assign_api import CouponAssignmentHandler
44
from sheets.exceptions import CouponAssignmentError
5-
from sheets.api import ExpandedSheetsClient, get_authorized_pygsheets_client
6-
from sheets.utils import google_date_string_to_datetime, spreadsheet_repr
7-
from ecommerce.models import BulkCouponAssignment
85

96

107
def get_assignment_spreadsheet_by_title(pygsheets_client, title):
@@ -31,60 +28,3 @@ def get_assignment_spreadsheet_by_title(pygsheets_client, title):
3128
f"{len(matching_spreadsheets)} were found."
3229
)
3330
return matching_spreadsheets[0]
34-
35-
36-
def assign_coupons_from_spreadsheet(
37-
use_sheet_id: bool, value: str, force: bool = False
38-
):
39-
"""
40-
Fetches and processes a coupon assignment spreadsheet using either the sheet ID or title.
41-
42-
Args:
43-
use_sheet_id (bool): If True, 'value' represents the spreadsheet ID; otherwise, it represents the title.
44-
value (str): The spreadsheet ID or title.
45-
46-
Returns:
47-
dict: A dictionary containing the result of the processing.
48-
"""
49-
50-
if not value:
51-
raise CouponAssignmentError("Spreadsheet identifier (ID or Title) is required.")
52-
53-
pygsheets_client = get_authorized_pygsheets_client()
54-
55-
# Fetch the correct spreadsheet
56-
if use_sheet_id:
57-
spreadsheet = pygsheets_client.open_by_key(value)
58-
else:
59-
spreadsheet = get_assignment_spreadsheet_by_title(pygsheets_client, value)
60-
61-
expanded_sheets_client = ExpandedSheetsClient(pygsheets_client)
62-
metadata = expanded_sheets_client.get_drive_file_metadata(
63-
file_id=spreadsheet.id, fields="modifiedTime"
64-
)
65-
sheet_last_modified = google_date_string_to_datetime(metadata["modifiedTime"])
66-
67-
bulk_assignment, created = BulkCouponAssignment.objects.get_or_create(
68-
assignment_sheet_id=spreadsheet.id
69-
)
70-
71-
if (
72-
bulk_assignment.sheet_last_modified_date
73-
and sheet_last_modified <= bulk_assignment.sheet_last_modified_date
74-
and not force
75-
):
76-
raise CouponAssignmentError(
77-
f"Spreadsheet is unchanged since last processed ({spreadsheet_repr(spreadsheet)}, last modified: {sheet_last_modified.isoformat()})."
78-
)
79-
80-
coupon_assignment_handler = CouponAssignmentHandler(
81-
spreadsheet_id=spreadsheet.id, bulk_assignment=bulk_assignment
82-
)
83-
84-
bulk_assignment, num_created, num_removed = (
85-
coupon_assignment_handler.process_assignment_spreadsheet()
86-
)
87-
bulk_assignment.sheet_last_modified_date = sheet_last_modified
88-
bulk_assignment.save()
89-
90-
return spreadsheet_repr(spreadsheet), num_created, num_removed, bulk_assignment.id

‎sheets/management/utils_test.py

+3-119
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,11 @@
11
"""Tests for sheets.management.utils"""
22

3-
from unittest.mock import MagicMock, patch
4-
53
import pytest
6-
from datetime import timedelta
74

8-
from sheets.management import utils
9-
from mitxpro.utils import now_in_utc
5+
from unittest.mock import MagicMock
106

11-
from sheets.management.utils import (
12-
assign_coupons_from_spreadsheet,
13-
CouponAssignmentError,
14-
)
15-
from ecommerce.factories import BulkCouponAssignmentFactory
7+
from sheets.management import utils
8+
from sheets.management.utils import CouponAssignmentError
169

1710

1811
@pytest.fixture(name="valid_enum_rows")
@@ -58,112 +51,3 @@ def test_get_assignment_sheet_by_title_multiple():
5851
)
5952
with pytest.raises(CouponAssignmentError):
6053
utils.get_assignment_spreadsheet_by_title(mock_pygsheets_client, "fake")
61-
62-
63-
@pytest.mark.django_db
64-
@pytest.mark.parametrize(
65-
"use_sheet_id, value, force, sheet_modified_offset, existing_modified_offset, expect_error, force_processing",
66-
[
67-
(
68-
True,
69-
"sheet-id-123",
70-
False,
71-
timedelta(days=-1),
72-
timedelta(days=-2),
73-
False,
74-
True,
75-
), # Valid, modified after last
76-
(
77-
False,
78-
"fake-title",
79-
False,
80-
timedelta(days=-2),
81-
timedelta(days=-1),
82-
True,
83-
False,
84-
), # No modification
85-
(
86-
True,
87-
"sheet-id-123",
88-
True,
89-
timedelta(days=-2),
90-
timedelta(days=-1),
91-
False,
92-
True,
93-
), # Forced processing
94-
(
95-
False,
96-
"",
97-
False,
98-
None,
99-
None,
100-
True,
101-
False,
102-
), # Missing value should raise an error
103-
],
104-
)
105-
@patch("sheets.management.utils.get_authorized_pygsheets_client")
106-
@patch("sheets.management.utils.get_assignment_spreadsheet_by_title")
107-
@patch("sheets.management.utils.google_date_string_to_datetime")
108-
@patch("sheets.management.utils.CouponAssignmentHandler")
109-
def test_assign_coupons_from_spreadsheet(
110-
mock_coupon_assignment_handler,
111-
mock_google_date_string_to_datetime,
112-
mock_get_assignment_sheet_by_title,
113-
mock_get_authorized_pygsheets_client,
114-
use_sheet_id,
115-
value,
116-
force,
117-
sheet_modified_offset,
118-
existing_modified_offset,
119-
expect_error,
120-
force_processing,
121-
):
122-
"""Test assign_coupons_from_spreadsheet with different conditions."""
123-
124-
mock_pygsheets_client = MagicMock()
125-
mock_get_authorized_pygsheets_client.return_value = mock_pygsheets_client
126-
127-
mock_spreadsheet = MagicMock()
128-
129-
# Title and ID should be assigned to assert the returning value of assign_coupons_from_spreadsheet
130-
mock_spreadsheet.title = "fake-title"
131-
mock_spreadsheet.id = "sheet-id-123"
132-
133-
if use_sheet_id:
134-
mock_pygsheets_client.open_by_key.return_value = mock_spreadsheet
135-
else:
136-
mock_get_assignment_sheet_by_title.return_value = mock_spreadsheet
137-
138-
sheet_last_modified = (
139-
now_in_utc() + sheet_modified_offset if sheet_modified_offset else None
140-
)
141-
mock_google_date_string_to_datetime.return_value = sheet_last_modified
142-
143-
bulk_assignment = BulkCouponAssignmentFactory.create(
144-
assignment_sheet_id="sheet-id-123",
145-
sheet_last_modified_date=now_in_utc() + existing_modified_offset
146-
if existing_modified_offset
147-
else None,
148-
)
149-
150-
if expect_error:
151-
with pytest.raises(CouponAssignmentError):
152-
assign_coupons_from_spreadsheet(use_sheet_id, value, force)
153-
else:
154-
mock_process_assignment = (
155-
mock_coupon_assignment_handler.return_value.process_assignment_spreadsheet
156-
)
157-
mock_process_assignment.return_value = (bulk_assignment, 5, 2)
158-
159-
result = assign_coupons_from_spreadsheet(use_sheet_id, value, force)
160-
if force_processing:
161-
assert result == (
162-
"'fake-title', id: sheet-id-123",
163-
5,
164-
2,
165-
bulk_assignment.id,
166-
)
167-
mock_process_assignment.assert_called_once()
168-
else:
169-
mock_process_assignment.assert_not_called()

‎sheets/utils.py

+62
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from django.conf import settings
1010
from django.urls import reverse
1111

12+
from ecommerce.models import BulkCouponAssignment
1213
from mitxpro.utils import matching_item_index
1314
from sheets.constants import (
1415
ASSIGNMENT_SHEET_PREFIX,
@@ -24,6 +25,7 @@
2425
WORKSHEET_TYPE_DEFERRAL,
2526
WORKSHEET_TYPE_REFUND,
2627
)
28+
from sheets.exceptions import CouponAssignmentError
2729

2830

2931
def generate_google_client_config():
@@ -630,3 +632,63 @@ def build_drive_file_email_share_request(file_id, email_to_share):
630632
supportsTeamDrives=True,
631633
**added_kwargs,
632634
)
635+
636+
637+
def assign_coupons_from_spreadsheet(
638+
use_sheet_id: bool, value: str, force: bool = False
639+
):
640+
"""
641+
Fetches and processes a coupon assignment spreadsheet using either the sheet ID or title.
642+
643+
Args:
644+
use_sheet_id (bool): If True, 'value' represents the spreadsheet ID; otherwise, it represents the title.
645+
value (str): The spreadsheet ID or title.
646+
647+
Returns:
648+
dict: A dictionary containing the result of the processing.
649+
"""
650+
from sheets.api import ExpandedSheetsClient, get_authorized_pygsheets_client
651+
from sheets.coupon_assign_api import CouponAssignmentHandler
652+
from sheets.management.utils import get_assignment_spreadsheet_by_title
653+
654+
if not value:
655+
raise CouponAssignmentError("Spreadsheet identifier (ID or Title) is required.")
656+
657+
pygsheets_client = get_authorized_pygsheets_client()
658+
659+
# Fetch the correct spreadsheet
660+
if use_sheet_id:
661+
spreadsheet = pygsheets_client.open_by_key(value)
662+
else:
663+
spreadsheet = get_assignment_spreadsheet_by_title(pygsheets_client, value)
664+
665+
expanded_sheets_client = ExpandedSheetsClient(pygsheets_client)
666+
metadata = expanded_sheets_client.get_drive_file_metadata(
667+
file_id=spreadsheet.id, fields="modifiedTime"
668+
)
669+
sheet_last_modified = google_date_string_to_datetime(metadata["modifiedTime"])
670+
671+
bulk_assignment, created = BulkCouponAssignment.objects.get_or_create(
672+
assignment_sheet_id=spreadsheet.id
673+
)
674+
675+
if (
676+
bulk_assignment.sheet_last_modified_date
677+
and sheet_last_modified <= bulk_assignment.sheet_last_modified_date
678+
and not force
679+
):
680+
raise CouponAssignmentError(
681+
f"Spreadsheet is unchanged since last processed ({spreadsheet_repr(spreadsheet)}, last modified: {sheet_last_modified.isoformat()})."
682+
)
683+
684+
coupon_assignment_handler = CouponAssignmentHandler(
685+
spreadsheet_id=spreadsheet.id, bulk_assignment=bulk_assignment
686+
)
687+
688+
bulk_assignment, num_created, num_removed = (
689+
coupon_assignment_handler.process_assignment_spreadsheet()
690+
)
691+
bulk_assignment.sheet_last_modified_date = sheet_last_modified
692+
bulk_assignment.save()
693+
694+
return spreadsheet_repr(spreadsheet), num_created, num_removed, bulk_assignment.id

0 commit comments

Comments
 (0)