diff --git a/.gitignore b/.gitignore index 8eb1e5e..178f4ff 100644 --- a/.gitignore +++ b/.gitignore @@ -120,4 +120,5 @@ ___* # local docker settings and config .docker/* -!.docker/.gitkeep \ No newline at end of file +!.docker/.gitkeep +*.db diff --git a/AGENTS.md b/AGENTS.md index 65f7ff5..d71d00d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,12 +18,67 @@ This repository contains AI coding instructions and architectural guidelines in: - `.github/copilot-instructions.md` Those instructions define: + - Coding style - Task conventions - Architectural constraints All agents and contributors MUST follow those rules when adding or modifying tasks. +## Testing and Celery Behavior + +### Custom test runner + +The project uses a custom Django test runner configured in [impresso/settings.py](impresso/settings.py#L104): + +- `TEST_RUNNER = "impresso.tests.test_runner.TestRunner"` + +The runner implementation is in [impresso/tests/test_runner.py](impresso/tests/test_runner.py#L1) and enables these Celery settings for tests: + +- `CELERY_TASK_ALWAYS_EAGER=True` +- `CELERY_TASK_EAGER_PROPAGATES=True` +- `CELERY_TASK_STORE_EAGER_RESULT=False` + +### Practical implications for tests + +- Calls to Celery queue APIs like `.delay()` and `.apply_async()` run inline during tests in this repository. +- Exceptions raised by task code propagate to the test process (because eager propagates is enabled). +- For scheduling logic, do not wait for wall-clock execution in tests. Instead, patch the enqueue call and assert arguments. + +### `apply_async(..., eta=...)` in tests + +When eager mode is enabled by the custom test runner, `apply_async(..., eta=...)` does not behave like a real delayed worker schedule. The call executes immediately in-process, while `eta` is still passed as metadata. + +For scheduling assertions, patch the task method and assert it was called with the expected `eta` and kwargs, for example: + +```python +from unittest.mock import patch + +with patch( + "impresso.tasks.userSpecialMembershipRequest_tasks.revoke_special_membership_request.apply_async" +) as mock_apply_async: + _schedule_temporary_revocation(req) + + mock_apply_async.assert_called_once() + _, call_kwargs = mock_apply_async.call_args + assert call_kwargs["kwargs"] == {"instance_id": req.pk} + assert "eta" in call_kwargs +``` + +### Temporary special membership revocation strategy + +Temporary special membership revocation is beat-driven in this repository: + +- Requests are marked with `temporary_expires_at` when temporarily approved. +- Periodic task `revoke_expired_temporary_memberships_beat` is the single revocation trigger. +- Save/update flows should not enqueue direct revocation scheduling for temporary requests. + +Testing implications for this feature: + +- Do not patch `revoke_special_membership_request.apply_async` to assert ETA scheduling in temporary auto-accept tests. +- Prefer asserting persisted state (`status`, `temporary_expires_at`) and beat-driven revocation behavior. +- Keep generic `apply_async(..., eta=...)` assertions only for features that still use explicit ETA scheduling. + ## Email and templates ### Overview @@ -79,4 +134,3 @@ send_magic_link_email( ``` The function appends `?token=` to `magic_link_callback_url` and passes the full URL as `magic_link` to the template context. - diff --git a/impresso/admin.py b/impresso/admin.py index cc4900b..d740577 100644 --- a/impresso/admin.py +++ b/impresso/admin.py @@ -7,7 +7,7 @@ from django.contrib.auth.models import User from django.utils.translation import ngettext from django.utils import timezone - +from django.utils.html import format_html from impresso.models.userBitmapSubscription import UserBitmapSubscription from .models import Issue, Job, Page, Newspaper from .models import SearchQuery, ContentItem @@ -87,16 +87,23 @@ def set_terms_accepted_date(self, request, queryset): class SpecialMembershipDatasetAdminForm(forms.ModelForm): + class Meta: model = SpecialMembershipDataset fields = "__all__" + help_texts = { + "metadata": format_html( + "Allowed Values: 
{}

Note: revokeAfterDays is working only if enableTemporaryAutomaticAcceptance is set to true.", + ", ".join(sorted(SpecialMembershipDataset.METADATA_ALLOWED_KEYS)), + ), + } def clean_metadata(self) -> dict: metadata = self.cleaned_data.get("metadata") or {} if not isinstance(metadata, dict): raise ValidationError("Metadata must be a JSON object.") - allowed_keys = {"modality"} + allowed_keys = SpecialMembershipDataset.METADATA_ALLOWED_KEYS extra_keys = set(metadata.keys()) - allowed_keys if extra_keys: raise ValidationError( @@ -115,18 +122,75 @@ def clean_metadata(self) -> dict: if modality not in allowed_modalities: allowed = ", ".join(sorted(allowed_modalities)) raise ValidationError(f"metadata.modality must be one of: {allowed}.") - + enable_temporary_automatic_acceptance = metadata.get( + "enableTemporaryAutomaticAcceptance" + ) + if enable_temporary_automatic_acceptance is not None and not isinstance( + enable_temporary_automatic_acceptance, bool + ): + raise ValidationError( + "metadata.enableTemporaryAutomaticAcceptance must be a boolean." + ) + revoke_after_days = metadata.get("revokeAfterDays") + if revoke_after_days is not None: + if not isinstance(revoke_after_days, (int, float)): + raise ValidationError( + "metadata.revokeAfterDays must be an integer or float." + ) + if revoke_after_days <= 0: + raise ValidationError( + "metadata.revokeAfterDays must be a positive integer or float." + ) return metadata @admin.register(SpecialMembershipDataset) class SpecialMembershipDatasetAdmin(ModelAdmin): - list_display = ("title", "bitmap_position", "reviewer", "metadata") + list_display = ( + "title", + "bitmap_position", + "reviewer", + "modality", + "enable_temporary_automatic_acceptance", + "revoke_after_days_display", + ) search_fields = ["title", "reviewer__username", "reviewer__email"] readonly_fields = ("bitmap_position",) autocomplete_fields = ["reviewer"] form = SpecialMembershipDatasetAdminForm + @admin.display(description="Revoke after") + def revoke_after_days_display(self, obj: SpecialMembershipDataset) -> str: + days = obj.revoke_after_days + if days is None: + return "-" + total_minutes = round(days * 24 * 60) + parts = [] + years, remainder = divmod(total_minutes, 365 * 24 * 60) + if years: + parts.append(f"{years}y") + months, remainder = divmod(remainder, 30 * 24 * 60) + if months: + parts.append(f"{months}mo") + weeks, remainder = divmod(remainder, 7 * 24 * 60) + if weeks: + parts.append(f"{weeks}w") + day_part, remainder = divmod(remainder, 24 * 60) + if day_part: + parts.append(f"{day_part}d") + hours, minutes = divmod(remainder, 60) + if hours: + parts.append(f"{hours}h") + if minutes: + parts.append(f"{minutes}m") + return format_html( + ( + f"{obj.revoke_after_days} days
" + " ".join(parts) + if parts + else "0m" + ), + ) + @admin.register(Issue) class IssueAdmin(ModelAdmin): diff --git a/impresso/management/commands/checktemporarymemberships.py b/impresso/management/commands/checktemporarymemberships.py new file mode 100644 index 0000000..c4a598e --- /dev/null +++ b/impresso/management/commands/checktemporarymemberships.py @@ -0,0 +1,62 @@ +from typing import Any +from django.core.management.base import BaseCommand +from django.utils import timezone +from impresso.models import UserSpecialMembershipRequest +from impresso.tasks.userSpecialMembershipRequest_tasks import ( + revoke_expired_temporary_memberships, +) + +class Command(BaseCommand): + help = ( + "Check temporary special memberships and enqueue asynchronous revocation for " + "expired ones." + ) + + def add_arguments(self, parser) -> None: + parser.add_argument( + "--dry-run", + action="store_true", + help="Run the command without making any actual changes to the database.", + ) + + def handle(self, *args: Any, **options: Any) -> None: + dry_run = options.get("dry_run", False) + + if dry_run: + self.stdout.write(self.style.NOTICE("Running in DRY RUN mode.")) + + requests = UserSpecialMembershipRequest.objects.filter( + status=UserSpecialMembershipRequest.STATUS_APPROVED_TEMPORARY + ) + + count = requests.count() + self.stdout.write(self.style.SUCCESS(f"Found {count} special memberships with temporary approval.")) + + for req in requests: + expires_at = req.temporary_expires_at + expired_str = "" + if expires_at and expires_at < timezone.now(): + expired_str = " (EXPIRED)" + elif expires_at: + expired_str = f" (Expires: {expires_at.strftime('%Y-%m-%d %H:%M:%S %Z')})" + + dataset_title = req.subscription.title if req.subscription else "None" + self.stdout.write( + f"- Request ID: {req.pk}, User: {req.user.username}, " + f"Dataset: {dataset_title}{expired_str}" + ) + + if dry_run: + self.stdout.write( + self.style.NOTICE( + "Dry run completed: task dispatch skipped." + ) + ) + return + + async_result = revoke_expired_temporary_memberships.delay() + self.stdout.write( + self.style.SUCCESS( + f"Enqueued revoke_expired_temporary_memberships task (id={async_result.id})." + ) + ) diff --git a/impresso/management/commands/createspecialmembership.py b/impresso/management/commands/createspecialmembership.py index 4890c1e..43aedde 100644 --- a/impresso/management/commands/createspecialmembership.py +++ b/impresso/management/commands/createspecialmembership.py @@ -9,9 +9,7 @@ class Command(BaseCommand): - help = ( - "Create a pending special membership request for a specific user and dataset." - ) + help = "Create a special membership request for a specific user and dataset." def add_arguments(self, parser) -> None: parser.add_argument( diff --git a/impresso/management/commands/createspecialmembershiprequest.py b/impresso/management/commands/createspecialmembershiprequest.py new file mode 100644 index 0000000..ec1791a --- /dev/null +++ b/impresso/management/commands/createspecialmembershiprequest.py @@ -0,0 +1,123 @@ +from datetime import timedelta +from typing import Any + +from django.contrib.auth.models import User +from django.core.management.base import BaseCommand, CommandError +from django.db import IntegrityError +from django.utils import timezone + +from django.conf import settings +from impresso.models import SpecialMembershipDataset, UserSpecialMembershipRequest + + +class Command(BaseCommand): + help = ( + "Create a pending special membership request on behalf of a user " + "using user email and dataset id." + ) + + def add_arguments(self, parser) -> None: + parser.add_argument( + "user_email", + type=str, + help="Email of the user for whom the request will be created", + ) + parser.add_argument( + "dataset_id", + type=int, + help="SpecialMembershipDataset id", + ) + parser.add_argument( + "--status", + type=str, + default=UserSpecialMembershipRequest.STATUS_PENDING, + choices=[ + UserSpecialMembershipRequest.STATUS_PENDING, + UserSpecialMembershipRequest.STATUS_APPROVED, + UserSpecialMembershipRequest.STATUS_APPROVED_TEMPORARY, + UserSpecialMembershipRequest.STATUS_REJECTED, + UserSpecialMembershipRequest.STATUS_REVOKED, + ], + help=( + "Initial status of the request " + f"(default: {UserSpecialMembershipRequest.STATUS_PENDING})" + ), + ) + parser.add_argument( + "--notes", + type=str, + default=None, + help="Optional notes to attach to the request", + ) + parser.add_argument( + "--revoke-after", + type=float, + default=None, + dest="revoke_after", + help="Number of days after which the temporary membership expires (sets temporary_expires_at and overrides the default value from subscription metadata)", + ) + + def handle( + self, user_email: str, dataset_id: int, *args: Any, **options: Any + ) -> None: + try: + user = User.objects.get(email=user_email) + except User.DoesNotExist as exc: + raise CommandError( + f"User with email '{user_email}' does not exist." + ) from exc + + try: + dataset = SpecialMembershipDataset.objects.select_related("reviewer").get( + pk=dataset_id + ) + except SpecialMembershipDataset.DoesNotExist as exc: + raise CommandError( + f"SpecialMembershipDataset with id={dataset_id} does not exist." + ) from exc + + revoke_after: float | None = options["revoke_after"] + # Check that revoke_after options is valid int or float greater than 0 + if revoke_after is not None: + if revoke_after <= 0: + raise CommandError( + f"Invalid value for --revoke-after: {revoke_after}. It must be a positive number." + ) + + if ( + revoke_after is None + and options["status"] + == UserSpecialMembershipRequest.STATUS_APPROVED_TEMPORARY + ): + revoke_after = dataset.resolve_revoke_after_days( + default_days=settings.IMPRESSO_SPECIAL_MEMBERSHIP_TEMPORARY_APPROVAL_DEFAULT_DAYS + ) + + # normally this would be set from request.creation_date + revoke_after, + # but since we're creating the request now we can set it from now + revoke_after. + temporary_expires_at = ( + timezone.now() + timedelta(days=revoke_after) + if revoke_after is not None + else None + ) + + try: + request = UserSpecialMembershipRequest.objects.create( + user=user, + reviewer=dataset.reviewer, + subscription=dataset, + status=options["status"], + notes=options["notes"], + temporary_expires_at=temporary_expires_at, + ) + except IntegrityError as exc: + raise CommandError( + "A request for this user and dataset already exists or could not be created." + ) from exc + + self.stdout.write( + self.style.SUCCESS( + "Created special membership request " + f"id={request.pk} for user='{user.email}' and dataset_id={dataset.pk}." + ) + ) diff --git a/impresso/migrations/0058_remove_userrequest_subscription_and_more.py b/impresso/migrations/0058_remove_userrequest_subscription_and_more.py new file mode 100644 index 0000000..8fad295 --- /dev/null +++ b/impresso/migrations/0058_remove_userrequest_subscription_and_more.py @@ -0,0 +1,185 @@ +# Generated by Django 5.2.10 on 2026-04-21 14:05 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ( + "impresso", + "0056_collection_search_query_hash_squashed_0057_remove_collection_search_query_hash_and_more", + ), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.SeparateDatabaseAndState( + database_operations=[], + state_operations=[ + migrations.CreateModel( + name="SpecialMembershipDataset", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("title", models.CharField(db_column="name", max_length=255)), + ( + "bitmap_position", + models.PositiveIntegerField( + blank=True, null=True, unique=True + ), + ), + ("metadata", models.JSONField(blank=True, default=dict)), + ( + "reviewer", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="reviewed_datasets", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "verbose_name": "Special Membership Access", + "verbose_name_plural": "Special Membership Accesses", + "db_table": "impresso_datasetbitmapposition", + }, + ), + migrations.CreateModel( + name="UserBitmapSubscription", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "specialmembershipdataset", + models.ForeignKey( + db_column="datasetbitmapposition_id", + on_delete=django.db.models.deletion.CASCADE, + to="impresso.specialmembershipdataset", + ), + ), + ( + "userbitmap", + models.ForeignKey( + db_column="userbitmap_id", + on_delete=django.db.models.deletion.CASCADE, + to="impresso.userbitmap", + ), + ), + ], + options={ + "verbose_name": "User Special Membership Access", + "verbose_name_plural": "User Special Membership Accesses", + "db_table": "impresso_userbitmap_subscriptions", + "unique_together": {("userbitmap", "specialmembershipdataset")}, + }, + ), + migrations.CreateModel( + name="UserSpecialMembershipRequest", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("date_created", models.DateTimeField(auto_now_add=True)), + ("date_last_modified", models.DateTimeField(auto_now=True)), + ( + "status", + models.CharField( + choices=[ + ("pending", "Pending"), + ("approved", "Approved"), + ("temporary", "Approved (Temporary)"), + ("rejected", "Rejected"), + ("revoked", "Revoked"), + ], + default="pending", + max_length=10, + ), + ), + ( + "changelog", + models.JSONField(blank=True, default=list, null=True), + ), + ("notes", models.TextField(blank=True, null=True)), + ( + "reviewer", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="review", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "subscription", + models.ForeignKey( + help_text="The special membership dataset being requested", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="impresso.specialmembershipdataset", + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="request", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "verbose_name": "User Special Membership Request", + "verbose_name_plural": "User Special Membership Requests", + "db_table": "impresso_userrequest", + "unique_together": {("user", "subscription")}, + }, + ), + migrations.AlterField( + model_name="userbitmap", + name="subscriptions", + field=models.ManyToManyField( + blank=True, + through="impresso.UserBitmapSubscription", + to="impresso.specialmembershipdataset", + ), + ), + migrations.DeleteModel(name="DatasetBitmapPosition"), + migrations.DeleteModel(name="UserRequest"), + ], + ), + migrations.AddField( + model_name="userspecialmembershiprequest", + name="temporary_expires_at", + field=models.DateTimeField( + blank=True, + help_text="Expiration date used for temporary automatic approvals", + null=True, + ), + ), + ] diff --git a/impresso/migrations/0059_alter_collection_serialized_search_query_and_more.py b/impresso/migrations/0059_alter_collection_serialized_search_query_and_more.py new file mode 100644 index 0000000..9a75531 --- /dev/null +++ b/impresso/migrations/0059_alter_collection_serialized_search_query_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 5.2.10 on 2026-05-07 12:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('impresso', '0058_remove_userrequest_subscription_and_more'), + ] + + operations = [ + migrations.AlterField( + model_name='collection', + name='serialized_search_query', + field=models.TextField(blank=True, help_text='Initial search query that generated the collection.', null=True), + ), + migrations.AlterField( + model_name='searchquery', + name='hash', + field=models.TextField(blank=True, null=True), + ), + ] diff --git a/impresso/models/specialMembershipDataset.py b/impresso/models/specialMembershipDataset.py index 9bd1c1c..f8becc3 100644 --- a/impresso/models/specialMembershipDataset.py +++ b/impresso/models/specialMembershipDataset.py @@ -1,5 +1,6 @@ from typing import Optional, TypedDict - +from numbers import Real +from django.conf import settings from django.db import models from django.db.models import Max @@ -11,6 +12,8 @@ class Metadata(TypedDict, total=False): """ modality: Optional[str] # e.g., "cc_reviewer", "notify_reviewer" + enableTemporaryAutomaticAcceptance: Optional[bool] + revokeAfterDays: Optional[float] class SpecialMembershipDataset(models.Model): @@ -41,6 +44,7 @@ class SpecialMembershipDataset(models.Model): blank=True, ) metadata: Metadata = models.JSONField(default=dict, blank=True) + reviewer = models.ForeignKey( "auth.User", on_delete=models.SET_NULL, @@ -49,9 +53,57 @@ class SpecialMembershipDataset(models.Model): blank=True, ) + METADATA_ALLOWED_KEYS = { + "modality", + "enableTemporaryAutomaticAcceptance", + "revokeAfterDays", + } + def __str__(self): return self.title + @property + def modality(self) -> Optional[str]: + return self.metadata.get("modality") + + @property + def enable_temporary_automatic_acceptance(self) -> Optional[bool]: + value = self.metadata.get("enableTemporaryAutomaticAcceptance") + return bool(value) if value is not None else None + + @property + def revoke_after_days(self) -> Optional[float]: + value = self.metadata.get("revokeAfterDays") + return float(value) if value is not None else None + + def is_temporary_auto_accept_enabled(self) -> bool: + return bool(self.enable_temporary_automatic_acceptance) + + def is_modality_cc_reviewer_enabled(self) -> bool: + return ( + self.modality + == settings.IMPRESSO_EMAIL_MODALITY_SPECIAL_MEMBERSHIP_REQUEST_CC_REVIEWER + ) + + def resolve_revoke_after_days(self, default_days: Optional[float]) -> float: + """ + Returns the number of days after which a temporary approval + should be revoked. + + Priority: + 1. metadata["revokeAfterDays"] + 2. provided default_days + 3. Django settings fallback + """ + revoke_after_days = self.metadata.get("revokeAfterDays") + if isinstance(revoke_after_days, Real) and revoke_after_days > 0: + return float(revoke_after_days) + if isinstance(default_days, Real) and default_days > 0: + return float(default_days) + return float( + settings.IMPRESSO_SPECIAL_MEMBERSHIP_TEMPORARY_APPROVAL_DEFAULT_DAYS + ) + class Meta: verbose_name = "Special Membership Access" verbose_name_plural = "Special Membership Accesses" diff --git a/impresso/models/userSpecialMembershipRequest.py b/impresso/models/userSpecialMembershipRequest.py index 340cab9..b969757 100644 --- a/impresso/models/userSpecialMembershipRequest.py +++ b/impresso/models/userSpecialMembershipRequest.py @@ -3,7 +3,7 @@ from django.contrib.auth.models import User from .specialMembershipDataset import SpecialMembershipDataset from django.utils import timezone - +from datetime import timedelta # --- Typing Definition for Changelog Entry --- class ChangelogEntry(TypedDict): @@ -11,11 +11,14 @@ class ChangelogEntry(TypedDict): Defines the strict type structure for a special membership request changelog entry. """ - status: str # e.g., "pending", "approved", "rejected" + status: ( + str # e.g., "pending", "approved", "approved_temporary", "rejected", "revoked" + ) subscription: Optional[str] # The title of the subscription date: str # ISO formatted date string reviewer: Optional[str] # Username of the reviewer notes: str # Additional notes (guaranteed to be a string, not None) + temporary_expires_at: Optional[str] # ISO formatted date string or None class UserSpecialMembershipRequest(models.Model): @@ -23,18 +26,21 @@ class UserSpecialMembershipRequest(models.Model): UserRequest model represents a request made by a user for a subscription. Note: The unique_together constraint ensures that each user can only have one request per subscription, regardless of the reviewer. This is to prevent duplicate requests for the same subscription by the same user. - Check post_save_user_special_membership_request signal to handle the approval process. + Check `impresso.signals.post_save_user_special_membership_request` signal to handle the approval process. Attributes: STATUS_PENDING (str): Status indicating the request is pending. STATUS_APPROVED (str): Status indicating the request is approved. + STATUS_APPROVED_TEMPORARY (str): Status indicating the request is temporarily approved. STATUS_REJECTED (str): Status indicating the request is rejected. + STATUS_REVOKED (str): Status indicating the request has been revoked after temporary approval. user (ForeignKey): Foreign key to the User model representing the user making the request. reviewer (ForeignKey): Foreign key to the User model representing the reviewer of the request. subscription (ForeignKey): Foreign key to the SpecialMembershipDataset model representing the subscription requested. date_created (DateTimeField): The date and time when the request was created. date_last_modified (DateTimeField): The date and time when the request was last modified. + temporary_expires_at (DateTimeField): The expiration date used for temporary automatic approvals. status (CharField): The current status of the request. changelog (JSONField): A list of changes made to the request. notes (TextField): Additional notes related to the request. @@ -52,8 +58,17 @@ class UserSpecialMembershipRequest(models.Model): STATUS_PENDING = "pending" STATUS_APPROVED = "approved" + STATUS_APPROVED_TEMPORARY = "temporary" STATUS_REJECTED = "rejected" - + STATUS_REVOKED = "revoked" + # Define the choices for the status field using the class-level constants + STATUS_CHOICES = ( + (STATUS_PENDING, "Pending"), + (STATUS_APPROVED, "Approved"), + (STATUS_APPROVED_TEMPORARY, "Approved (Temporary)"), + (STATUS_REJECTED, "Rejected"), + (STATUS_REVOKED, "Revoked"), + ) user = models.ForeignKey(User, on_delete=models.CASCADE, related_name="request") reviewer = models.ForeignKey( User, on_delete=models.SET_NULL, related_name="review", null=True, blank=True @@ -66,14 +81,15 @@ class UserSpecialMembershipRequest(models.Model): ) date_created = models.DateTimeField(auto_now_add=True) date_last_modified = models.DateTimeField(auto_now=True) + temporary_expires_at = models.DateTimeField( + null=True, + blank=True, + help_text="Expiration date used for temporary automatic approvals", + ) status = models.CharField( max_length=10, default=STATUS_PENDING, - choices=( - (STATUS_PENDING, "Pending"), - (STATUS_APPROVED, "Approved"), - (STATUS_REJECTED, "Rejected"), - ), + choices=STATUS_CHOICES, ) changelog = models.JSONField(null=True, blank=True, default=list) notes = models.TextField(null=True, blank=True) @@ -87,19 +103,51 @@ class Meta: verbose_name_plural = "User Special Membership Requests" db_table = "impresso_userrequest" - def save(self, *args: Any, **kwargs: Any) -> None: - changelog_entry: ChangelogEntry = { + def _append_changelog(self) -> None: + entry: ChangelogEntry = { "status": self.status, "subscription": self.subscription.title if self.subscription else None, - "date": ( - timezone.now().isoformat() - if not self.pk - else self.date_last_modified.isoformat() - ), + "date": timezone.now().isoformat(), "reviewer": self.reviewer.username if self.reviewer else None, - "notes": self.notes if self.notes else "", + "notes": self.notes or "", + "temporary_expires_at": ( + self.temporary_expires_at.isoformat() + if self.temporary_expires_at + else None + ), } - current_changelog: List[ChangelogEntry] = self.changelog or [] - current_changelog.append(changelog_entry) - self.changelog = current_changelog # Assign the updated list back + current_changelog = self.changelog or [] + latest_entry = current_changelog[-1] if current_changelog else None + # Avoid duplicate consecutive entries + if latest_entry: + comparable_fields = [ + "status", + "reviewer", + "notes", + "temporary_expires_at", + "subscription", + ] + is_same = all( + latest_entry.get(field) == entry.get(field) + for field in comparable_fields + ) + + if is_same: + return + self.changelog = current_changelog + [entry] + + def save(self, *args: Any, **kwargs: Any) -> None: + self._append_changelog() super().save(*args, **kwargs) + + def calculate_temporary_expiration( + self, revoke_after_days: float + ) -> timezone.datetime: + """ + Calculates the expiration date for temporary approvals from request creation date. + Supports fractional days (e.g., 0.5 for 12 hours). + Returns: + timezone.datetime: The calculated expiration datetime for temporary approvals. + """ + initial_datetime = self.date_created or timezone.now() + return initial_datetime + timedelta(days=revoke_after_days) diff --git a/impresso/settings.py b/impresso/settings.py index 80d4cc8..f69f918 100644 --- a/impresso/settings.py +++ b/impresso/settings.py @@ -383,6 +383,8 @@ (IMPRESSO_GROUP_USER_PLAN_EDUCATIONAL, IMPRESSO_GROUP_USER_PLAN_EDUCATIONAL_LABEL), (IMPRESSO_GROUP_USER_PLAN_RESEARCHER, IMPRESSO_GROUP_USER_PLAN_RESEARCHER_LABEL), ) + +IMPRESSO_SPECIAL_MEMBERSHIP_TEMPORARY_APPROVAL_DEFAULT_DAYS = 1 IMPRESSO_EMAIL_LABEL_DEFAULT_FROM_EMAIL = f"Impresso Team <{DEFAULT_FROM_EMAIL}>" IMPRESSO_EMAIL_SUBJECT_AFTER_USER_REGISTRATION_PLAN_BASIC = "Access to Impresso" @@ -414,9 +416,15 @@ IMPRESSO_EMAIL_SUBJECT_AFTER_USER_SPECIAL_MEMBERSHIP_REQUEST_ACCEPTED_TO_USER = ( "Your Special Membership Request has been approved" ) +IMPRESSO_EMAIL_SUBJECT_AFTER_USER_SPECIAL_MEMBERSHIP_REQUEST_ACCEPTED_TEMPORARY_TO_USER = ( + "Your temporary Special Membership access has been approved" +) IMPRESSO_EMAIL_SUBJECT_AFTER_USER_SPECIAL_MEMBERSHIP_REQUEST_REJECTED_TO_USER = ( "Your Special Membership Request has been rejected" ) +IMPRESSO_EMAIL_SUBJECT_AFTER_USER_SPECIAL_MEMBERSHIP_REQUEST_REVOKED_TO_USER = ( + "Your temporary Special Membership access has expired" +) IMPRESSO_EMAIL_SUBJECT_AFTER_USER_ACTIVATION_PLAN_REJECTED_TO_USER = ( "Access granted to Impresso (Basic User Access)" diff --git a/impresso/tasks/userSpecialMembershipRequest_tasks.py b/impresso/tasks/userSpecialMembershipRequest_tasks.py index 87aeca7..2ef37f2 100644 --- a/impresso/tasks/userSpecialMembershipRequest_tasks.py +++ b/impresso/tasks/userSpecialMembershipRequest_tasks.py @@ -1,9 +1,6 @@ -from typing import Dict from celery.utils.log import get_task_logger -from django.db.utils import IntegrityError - -from impresso.models.specialMembershipDataset import SpecialMembershipDataset -from celery import shared_task +from django.utils import timezone +from django.conf import settings from ..celery import app from ..models.userSpecialMembershipRequest import UserSpecialMembershipRequest from impresso.utils.tasks.userSpecialMembershipRequest import ( @@ -15,6 +12,43 @@ logger = get_task_logger(__name__) +def _is_temporary_auto_accept_enabled(req: UserSpecialMembershipRequest) -> bool: + if not req.subscription: + return False + return req.subscription.is_temporary_auto_accept_enabled() + + +def _is_modality_cc_reviewer_enabled(req: UserSpecialMembershipRequest) -> bool: + if not req.subscription: + return False + return req.subscription.is_modality_cc_reviewer_enabled() + + +def _resolve_revoke_after_days(req: UserSpecialMembershipRequest) -> float: + """ + Always return a positive number of days from the request's subscription metadata. + If they're wrong or missing, get the value from the default settings.IMPRESSO_SPECIAL_MEMBERSHIP_TEMPORARY_APPROVAL_DEFAULT_DAYS + """ + req.refresh_from_db() # ensure we have the latest subscription metadata + DEFAULT_DAYS: float = ( + settings.IMPRESSO_SPECIAL_MEMBERSHIP_TEMPORARY_APPROVAL_DEFAULT_DAYS + ) + if not req.subscription: + logger.warning( + f"[instance:{req.pk}] No subscription associated with the request, cannot get revokeAfterDays, using default {DEFAULT_DAYS} day(s)" + ) + return DEFAULT_DAYS + + revoke_after_days = req.subscription.resolve_revoke_after_days(DEFAULT_DAYS) + if revoke_after_days != float(DEFAULT_DAYS): + return revoke_after_days + + logger.warning( + f"[instance:{req.pk}] revokeAfterDays is missing or invalid in subscription metadata, using default {DEFAULT_DAYS} day(s)" + ) + return revoke_after_days + + @app.task( bind=True, autoretry_for=(Exception,), @@ -24,14 +58,16 @@ ) def after_special_membership_request_created(self, instance_id: int) -> None: """ - THe request has been created outside of the flow in the admin panel. - In this case we should manually apply the post-create actions. - + The special membership request (pk:{instance_id}) has already been created in the database. + In Django Admin site, this task is scheduled when the `impresso.signals.post_save_user_special_membership_request` signal is triggered; + in other backends, this task must be called manually using the backend Celery integration. + Note that this task can be called with a status other than PENDING, thus triggering the + `after_special_membership_request_updated` logic immediately and skipping the + `send_email_after_user_special_membership_request_created` logic. Args: self: The task instance. instance_id (int): The ID of the UserSpecialMembershipRequest instance that was created. """ - # get request try: req = UserSpecialMembershipRequest.objects.get(pk=instance_id) except UserSpecialMembershipRequest.DoesNotExist: @@ -40,7 +76,37 @@ def after_special_membership_request_created(self, instance_id: int) -> None: logger.info( f"[UserSpecialMembershipRequest:{instance_id}] triggered signals after_special_membership_request_created, for user={req.user.username} subscription={req.subscription.title if req.subscription else 'None'} status={req.status}" ) + # If the request is pending and temporary auto-accept is enabled, approve it as temporary. + # Expired temporary memberships are later revoked by a dedicated Celery task, + # typically triggered from a cron-scheduled management command. + # instead of going through the regular pending flow. + if ( + req.status == UserSpecialMembershipRequest.STATUS_PENDING + and _is_temporary_auto_accept_enabled(req) + ): + # Always set an expires at date in case of temporary approval + revoke_after_days = _resolve_revoke_after_days(req) + req.status = UserSpecialMembershipRequest.STATUS_APPROVED_TEMPORARY + # add the temporary expiration date using the last modified date + revoke_after_days + req.temporary_expires_at = req.calculate_temporary_expiration(revoke_after_days) + req.save() + logger.info( + f"[instance:{instance_id}] auto-accepted as temporary for {revoke_after_days} day(s)" + ) + # The save above triggers the update task via signal, which applies bitmap + # and sends the temporary-approval email. + return + + # Apply special membership to bitmap before sending emails, so that the user gets the access as soon as they receive the email apply_special_membership_to_bitmap(instance=req, created=True, logger=logger) + + if req.status != UserSpecialMembershipRequest.STATUS_PENDING: + logger.info( + f"[instance:{instance_id}] created with status {req.status}, skipping created email and sending update email instead" + ) + after_special_membership_request_updated.delay(instance_id=instance_id) + return + send_email_after_user_special_membership_request_created( instance=req, logger=logger ) @@ -56,10 +122,14 @@ def after_special_membership_request_created(self, instance_id: int) -> None: def after_special_membership_request_updated(self, instance_id: int) -> None: """ Send emails after a UserSpecialMembershipRequest is updated. - TODO: - If its status is "STATUS_APPROVED", notify the user with the details of the special membership granted. - If its status is "STATUS_PENDING", notify the institution reviewer with the details of the request AND to the user that their request is pending review. - If its status is "STATUS_REJECTED", notify the user that their request has been rejected. + This task doesn't change the status of the request, but only reacts to the change by sending the appropriate email notifications and applying the special membership to the bitmap. + + In Django Admin site, this task is scheduled when the `impresso.signals.post_save_user_special_membership_request` signal is triggered and the instance is not newly created (i.e., it's an update); + in other backends, this task must be called manually using the backend Celery integration. + + If current status is "STATUS_APPROVED", congratulate the user with the details of the special membership granted. + If current status is "STATUS_PENDING", notify the institution reviewer with the details of the request AND to the user that their request is pending review. + If current status is "STATUS_REJECTED", notify the user that their request has been rejected. Args: self: The task instance. @@ -81,5 +151,76 @@ def after_special_membership_request_updated(self, instance_id: int) -> None: ) apply_special_membership_to_bitmap(instance=req, created=False, logger=logger) send_email_after_user_special_membership_request_updated( - instance=req, logger=logger + instance=req, + logger=logger, + is_modality_cc_reviewer_enabled=_is_modality_cc_reviewer_enabled(req), + ) + + +@app.task( + bind=True, + autoretry_for=(Exception,), + exponential_backoff=2, + retry_kwargs={"max_retries": 5}, + retry_jitter=True, +) +def revoke_special_membership_request(self, instance_id: int) -> None: + """ + Revoke a temporary special membership request when its expiration date is reached. + + Args: + self: The task instance. + instance_id (int): The ID of the UserSpecialMembershipRequest instance. + """ + try: + req = UserSpecialMembershipRequest.objects.get(pk=instance_id) + except UserSpecialMembershipRequest.DoesNotExist: + logger.error( + f"[instance:{instance_id}] Cannot revoke temporary access, request not found" + ) + return + + if req.status != UserSpecialMembershipRequest.STATUS_APPROVED_TEMPORARY: + logger.info( + f"[instance:{instance_id}] revocation skipped because status is {req.status}" + ) + return + + req.status = UserSpecialMembershipRequest.STATUS_REVOKED + req.save() + # usual signals will take care of applying the bitmap changes and sending the email notification about revocation + logger.info( + f"[instance:{instance_id}] temporary membership revoked for user={req.user.pk}" ) + + +@app.task( + bind=True, + autoretry_for=(Exception,), + exponential_backoff=2, + retry_kwargs={"max_retries": 5}, + retry_jitter=True, +) +def revoke_expired_temporary_memberships(self) -> None: + """ + Periodic task to revoke any STATUS_APPROVED_TEMPORARY memberships + that have passed their temporary_expires_at date. + """ + expired_requests = UserSpecialMembershipRequest.objects.filter( + status=UserSpecialMembershipRequest.STATUS_APPROVED_TEMPORARY, + temporary_expires_at__lt=timezone.now(), + ) + + count = expired_requests.count() + if count == 0: + logger.info("No expired temporary memberships found to revoke.") + return + + logger.info(f"Found {count} expired temporary memberships to revoke.") + + for req in expired_requests: + revoke_special_membership_request.delay(instance_id=req.pk) + + +# Backward-compatible alias for older imports/usages. +revoke_expired_temporary_memberships_beat = revoke_expired_temporary_memberships diff --git a/impresso/templates/emails/account_created_mailto_educational.html b/impresso/templates/emails/account_created_mailto_educational.html index 88f7487..8af230b 100644 --- a/impresso/templates/emails/account_created_mailto_educational.html +++ b/impresso/templates/emails/account_created_mailto_educational.html @@ -36,7 +36,8 @@

We hope you will enjoy working with Impresso and do let us know how you get on. To reach us, you can reply to this email, or join the Impresso community - Discord group + Discord group or click the + feedback button in the lower right of the interface.

diff --git a/impresso/templates/emails/account_created_mailto_researcher.html b/impresso/templates/emails/account_created_mailto_researcher.html index 88f7487..8af230b 100644 --- a/impresso/templates/emails/account_created_mailto_researcher.html +++ b/impresso/templates/emails/account_created_mailto_researcher.html @@ -36,7 +36,8 @@

We hope you will enjoy working with Impresso and do let us know how you get on. To reach us, you can reply to this email, or join the Impresso community - Discord group + Discord group or click the + feedback button in the lower right of the interface.

diff --git a/impresso/templates/emails/account_created_mailto_user.html b/impresso/templates/emails/account_created_mailto_user.html index 88f7487..8af230b 100644 --- a/impresso/templates/emails/account_created_mailto_user.html +++ b/impresso/templates/emails/account_created_mailto_user.html @@ -36,7 +36,8 @@

We hope you will enjoy working with Impresso and do let us know how you get on. To reach us, you can reply to this email, or join the Impresso community - Discord group + Discord group or click the + feedback button in the lower right of the interface.

diff --git a/impresso/templates/emails/pending_requests_reminder_to_reviewer.html b/impresso/templates/emails/pending_requests_reminder_to_reviewer.html index 5b1e86a..c844518 100644 --- a/impresso/templates/emails/pending_requests_reminder_to_reviewer.html +++ b/impresso/templates/emails/pending_requests_reminder_to_reviewer.html @@ -12,11 +12,11 @@