diff --git a/.github/workflows/deploy-docs.yml b/.github/workflows/deploy-docs.yml index a9fd57c1..db4d8ed5 100644 --- a/.github/workflows/deploy-docs.yml +++ b/.github/workflows/deploy-docs.yml @@ -55,7 +55,7 @@ jobs: run: cd frontend && npm ci - name: Build Storybook - run: cd frontend && npm run build-storybook -- --base=/ProgressRPG/storybook/ + run: cd frontend && npm run build-storybook - name: Combine sites run: | diff --git a/Makefile b/Makefile index 47affdfb..28341cf8 100755 --- a/Makefile +++ b/Makefile @@ -26,6 +26,8 @@ ds: t: docker compose exec web python manage.py test $(t) --keepdb --buffer +tnk: + docker compose exec web python manage.py test $(t) --buffer vt: cd ./frontend diff --git a/api/tests.py b/api/tests.py index 6a4d5fe8..8c0bac4a 100644 --- a/api/tests.py +++ b/api/tests.py @@ -32,7 +32,7 @@ def player_for(user) -> Player: class TestMeViewSet(APITestCase): def setUp(self): - self.character = Character.objects.create(given_name="Hero", can_link=True) + self.character = Character.objects.create(given_name="Hero") self.user = create_test_user( email="duncan@example.com", password="pass12345", @@ -194,32 +194,78 @@ def test_complete_onboarding_sets_flag(self): self.assertTrue(player_for(self.user).onboarding_completed) self.assertEqual(res.data, {"onboarding_completed": True}) - def test_today_points_null_when_no_active_link(self): + def test_daily_goals_null_when_no_active_link(self): self.authenticate() player = player_for(self.user) self.assertIsNone(player.active_link) - res = self.client.get(reverse("me-today-points")) + res = self.client.get(reverse("me-daily-goals")) self.assertEqual(res.status_code, status.HTTP_200_OK) - self.assertIsNone(res.data["points_today"]) + self.assertIsNone(res.data["goals"]) - def test_today_points_reflects_todays_completed_activities(self): + def test_daily_goals_all_false_when_linked_but_no_activity_or_login(self): self.authenticate() player = player_for(self.user) PlayerCharacterLink.objects.create(player=player, character=self.character) + res = self.client.get(reverse("me-daily-goals")) + + self.assertEqual(res.status_code, status.HTTP_200_OK) + goals = res.data["goals"] + self.assertFalse(goals["logged_in_today"]) + self.assertFalse(goals["completed_activity_today"]) + self.assertEqual(goals["activity_minutes_today"], 0) + self.assertFalse(goals["minutes_goal_met"]) + self.assertFalse(goals["all_goals_met"]) + self.assertFalse(goals["bonus_awarded_today"]) + self.assertEqual(goals["bonus_ap"], 0) + + def test_daily_goals_reflects_todays_completed_activities(self): + self.authenticate() + player = player_for(self.user) + PlayerCharacterLink.objects.create(player=player, character=self.character) + + PlayerActivity.objects.create( + player=player, + is_complete=True, + duration=1200, # 20 minutes + completed_at=datetime.now(timezone.utc), + ) + + res = self.client.get(reverse("me-daily-goals")) + + self.assertEqual(res.status_code, status.HTTP_200_OK) + goals = res.data["goals"] + self.assertTrue(goals["completed_activity_today"]) + self.assertEqual(goals["activity_minutes_today"], 20) + self.assertTrue(goals["minutes_goal_met"]) + + def test_daily_goals_all_met_awards_bonus_once(self): + from users.models import UserLogin + + self.authenticate() + player = player_for(self.user) + PlayerCharacterLink.objects.create(player=player, character=self.character) + UserLogin.objects.create(user=self.user) + PlayerActivity.objects.create( player=player, is_complete=True, - duration=1200, # 20 minutes -> 2 points + duration=1200, completed_at=datetime.now(timezone.utc), ) - res = self.client.get(reverse("me-today-points")) + res = self.client.get(reverse("me-daily-goals")) self.assertEqual(res.status_code, status.HTTP_200_OK) - self.assertEqual(res.data["points_today"], 2) + goals = res.data["goals"] + self.assertTrue(goals["all_goals_met"]) + # Reads never award the bonus themselves - only activity completion + # does (via check_and_award_daily_goals), so a plain GET here + # shouldn't have paid it out. + self.assertFalse(goals["bonus_awarded_today"]) + self.assertEqual(goals["bonus_ap"], 0) class CustomTokenObtainPairViewTests(APITestCase): diff --git a/api/views.py b/api/views.py index 65505a45..4d67fa11 100644 --- a/api/views.py +++ b/api/views.py @@ -363,24 +363,61 @@ def character(self, request): @extend_schema( responses=inline_serializer( - name="TodayPointsResponse", + name="DailyGoalsResponse", fields={ - "points_today": drf_serializers.IntegerField(allow_null=True), + "goals": inline_serializer( + name="DailyGoalsStateResponse", + fields={ + "logged_in_today": drf_serializers.BooleanField(), + "completed_activity_today": drf_serializers.BooleanField(), + "activity_minutes_today": drf_serializers.IntegerField(), + "minutes_goal_threshold": drf_serializers.IntegerField(), + "minutes_goal_met": drf_serializers.BooleanField(), + "all_goals_met": drf_serializers.BooleanField(), + "bonus_awarded_today": drf_serializers.BooleanField(), + "bonus_ap": drf_serializers.IntegerField(), + }, + allow_null=True, + ), }, ) ) @action(detail=False, methods=["get"]) - def today_points(self, request): + def daily_goals(self, request): """ - Personal "points earned today" for the map view's badge (issue #673). - `points_today` is null - not zero - when the player has no active + Daily goals for the map view's badge (issue #751, replacing the old + `today_points`/`points_today` mechanic from issue #673). `goals` is + null - not a set of all-false goals - when the player has no active PlayerCharacterLink, so the frontend can tell "no link" apart from - "linked but nothing earned yet today" and hide the badge entirely. + "linked but no goals cleared yet today" and hide the badge entirely. """ + from progression.daily_goals import ( + MINUTES_GOAL_THRESHOLD, + get_daily_goals_state, + ) + player = request.user.player link = player.active_link - return Response({"points_today": link.points_today if link else None}) + if not link: + return Response({"goals": None}) + + state = get_daily_goals_state(player) + + return Response( + { + "goals": { + "logged_in_today": state.logged_in_today, + "completed_activity_today": state.completed_activity_today, + "activity_minutes_today": state.activity_minutes_today, + "minutes_goal_threshold": MINUTES_GOAL_THRESHOLD, + "minutes_goal_met": state.minutes_goal_met, + "all_goals_met": state.all_goals_met, + "bonus_awarded_today": state.bonus_awarded_today, + "bonus_ap": state.bonus_ap, + } + } + ) @extend_schema( responses=inline_serializer( diff --git a/character/admin.py b/character/admin.py index d9c5a8bf..f8015d5b 100644 --- a/character/admin.py +++ b/character/admin.py @@ -60,6 +60,27 @@ def get_other_members(self, obj): return ", ".join(str(c) for c in others) +class CanLinkListFilter(admin.SimpleListFilter): + """ + can_link is a derived property, not a DB column, so it can't be listed + in list_filter directly - filter via Character.objects.linkable() + (the queryset-level equivalent) instead. + """ + + title = "can link" + parameter_name = "can_link" + + def lookups(self, request, model_admin): + return (("yes", "Yes"), ("no", "No")) + + def queryset(self, request, queryset): + if self.value() == "yes": + return queryset.filter(pk__in=Character.objects.linkable()) + if self.value() == "no": + return queryset.exclude(pk__in=Character.objects.linkable()) + return queryset + + @admin.action(description="Mark selected characters as NPCs and unlink from players") def mark_as_npc(modeladmin, request, queryset): for character in queryset: @@ -73,17 +94,6 @@ def mark_as_npc(modeladmin, request, queryset): ) -@admin.action(description="Mark selected characters as available to link") -def mark_as_canlink(modeladmin, request, queryset): - for character in queryset: - character.can_link = True - character.save(update_fields=["can_link"]) - - messages.success( - request, f"{queryset.count()} character(s) marked as available to link." - ) - - @admin.register(Character) class CharacterAdmin(admin.ModelAdmin): fieldsets = ( @@ -93,6 +103,7 @@ class CharacterAdmin(admin.ModelAdmin): "fields": ( "given_name", "can_link", + "is_reserved", "sex", ) }, @@ -145,7 +156,8 @@ class CharacterAdmin(admin.ModelAdmin): "birth_date", ] list_filter = [ - "can_link", + CanLinkListFilter, + "is_reserved", "birth_date", "death_date", "sex", @@ -156,6 +168,7 @@ class CharacterAdmin(admin.ModelAdmin): "links__player__name", ] readonly_fields = [ + "can_link", "get_player", "get_age", "created_at", @@ -168,7 +181,7 @@ class CharacterAdmin(admin.ModelAdmin): CharacterRelationshipMembershipInline, CharacterCurrencyInline, ] - actions = [mark_as_npc, mark_as_canlink] + actions = [mark_as_npc] @admin.display(description="Player") def get_player(self, obj): diff --git a/character/filters.py b/character/filters.py index c8a3b864..59d7127f 100644 --- a/character/filters.py +++ b/character/filters.py @@ -7,7 +7,7 @@ class CharacterFilter(django_filters.FilterSet): level = django_filters.RangeFilter(field_name="level") xp = django_filters.RangeFilter(field_name="xp") is_npc = django_filters.BooleanFilter(method="filter_is_npc") - can_link = django_filters.BooleanFilter(field_name="can_link") + can_link = django_filters.BooleanFilter(method="filter_can_link") class Meta: model = Character @@ -21,3 +21,15 @@ def filter_is_npc(self, queryset, name, value): else: # is_npc=False means HAS active player link return queryset.filter(links__is_active=True) + + def filter_can_link(self, queryset, name, value): + """ + can_link is derived, not a DB column - defer to + Character.objects.linkable(), the queryset-level equivalent of + Character.can_link, instead of duplicating its logic here. + """ + linkable = Character.objects.linkable() + if value: + return queryset.filter(pk__in=linkable) + else: + return queryset.exclude(pk__in=linkable) diff --git a/character/migrations/0021_remove_character_can_link_character_is_reserved.py b/character/migrations/0021_remove_character_can_link_character_is_reserved.py new file mode 100644 index 00000000..c74dd925 --- /dev/null +++ b/character/migrations/0021_remove_character_can_link_character_is_reserved.py @@ -0,0 +1,26 @@ +# Generated by Django 5.2.17 on 2026-08-13 16:31 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("character", "0020_remove_character_building"), + ] + + operations = [ + migrations.RemoveField( + model_name="character", + name="can_link", + ), + migrations.AddField( + model_name="character", + name="is_reserved", + field=models.BooleanField( + default=False, + help_text="Manually held back from linking (e.g. reserved for a future storyline), independent of age or link status.", + verbose_name="Reserved", + ), + ), + ] diff --git a/character/models/character.py b/character/models/character.py index fab8ffc6..c5ef0ad1 100644 --- a/character/models/character.py +++ b/character/models/character.py @@ -1,4 +1,5 @@ # from datetime import datetime +from datetime import timedelta from celery import current_app from decimal import Decimal from django.contrib.gis.geos import Point @@ -309,6 +310,33 @@ def get_miscarriage_change(self): ######################################################################## +class CharacterQuerySet(models.QuerySet): + def linkable(self): + """ + Characters currently eligible to be linked to a player - the + queryset-level equivalent of `Character.can_link`, for call sites + that need to filter/exist-check in SQL rather than load instances. + Keep this in sync with `Character.can_link` by hand; there's no + way to share the logic verbatim since one runs in the DB and the + other in Python. + """ + cutoff_date = timezone.now().date() - timedelta( + days=Character.MIN_LINK_AGE_DAYS + ) + return ( + self.filter(is_reserved=False) + .filter( + models.Q(birth_date__isnull=True) + | models.Q(birth_date__lte=cutoff_date) + ) + .exclude(links__is_active=True) + ) + + +class CharacterManager(models.Manager.from_queryset(CharacterQuerySet)): + pass + + class Character(LevelProgressionMixin, LifeCycleMixin, Movable): class SexChoices(models.TextChoices): MALE = "Male", "Male" @@ -335,11 +363,21 @@ class SexChoices(models.TextChoices): max_length=20, choices=SexChoices.choices, null=True, blank=True ) reputation = models.IntegerField(default=0) - can_link = models.BooleanField(default=False) + is_reserved = models.BooleanField( + default=False, + verbose_name="Reserved", + help_text="Manually held back from linking (e.g. reserved for a future storyline), independent of age or link status.", + ) link_points_multiplier = models.DecimalField( max_digits=5, decimal_places=2, default="1.00" ) + objects = CharacterManager() + + # Minimum age (in days) for a character to be linkable. ~18 years, + # matching spawn-time character generation. + MIN_LINK_AGE_DAYS = int(18 * 365.25) + @property def is_npc(self): """ @@ -347,6 +385,33 @@ def is_npc(self): """ return not self.links.filter(is_active=True).exists() + @property + def is_underage(self): + # An unknown birth_date isn't evidence of being underage - it just + # means age isn't gating linkability for this character. + if self.birth_date is None: + return False + return self.get_age() < self.MIN_LINK_AGE_DAYS + + @property + def can_link(self) -> bool: + """ + Whether this character is currently eligible to be linked to a + player, derived from independent reasons - manual reservation, + age, active link, and (once population centres can gate linking - + see #681) population_centre.characters_can_link - rather than a + flag several call sites could clobber. Keep in sync by hand with + `CharacterQuerySet.linkable`, the SQL-level equivalent used where + a Python loop over instances isn't practical. + """ + if self.is_reserved: + return False + if self.is_underage: + return False + if self.links.filter(is_active=True).exists(): + return False + return True + def __str__(self): return self.name @@ -520,34 +585,6 @@ def link_points(self): ) return int(base_points * multiplier) - @property - def player_time_today(self): - """ - Completed activity time for this link so far today (in minutes) - - a date-filtered variant of player_time, bounded to the current UTC - day instead of the whole link lifetime. - """ - start_of_day = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) - start = max(start_of_day, self.linked_at) - - qs = self.player.activities.filter(is_complete=True, completed_at__gte=start) - if self.unlinked_at: - qs = qs.filter(completed_at__lte=self.unlinked_at) - - total_seconds = qs.aggregate(total=Sum("duration"))["total"] or 0 - return int(total_seconds // 60) - - @property - def points_today(self): - """ - Points earned today (issue #673's map-view "today" badge) - only the - activity-time component of link_points, date-filtered to today. - link_points' other terms (days_linked * 20, login_points) aren't - "earned today" in the same sense, so they're deliberately left out - here rather than prorated. - """ - return self.player_time_today // 10 - @classmethod def get_character(cls, player: Player) -> Character: return link_services.player_link_get_character(cls, player) diff --git a/character/services/character_services.py b/character/services/character_services.py index c2cd135b..6711fb12 100644 --- a/character/services/character_services.py +++ b/character/services/character_services.py @@ -40,6 +40,4 @@ def character_assign_work(character, building) -> None: def character_has_available(model_cls) -> bool: - return ( - model_cls.objects.filter(can_link=True).exclude(links__is_active=True).exists() - ) + return model_cls.objects.linkable().exists() diff --git a/character/services/link_services.py b/character/services/link_services.py index f0ff0816..3af344ba 100644 --- a/character/services/link_services.py +++ b/character/services/link_services.py @@ -37,8 +37,6 @@ def player_link_unlink(link) -> None: link.unlinked_at = timezone.now() link.is_active = False link.save() - link.character.can_link = True - link.character.save(update_fields=["can_link"]) def player_link_deactivate_active_links(model_cls, player) -> None: @@ -59,6 +57,4 @@ def player_link_assign_character(model_cls, player, character): character=character, is_active=True, ) - character.can_link = False - character.save(update_fields=["can_link"]) return link diff --git a/character/signals.py b/character/signals.py index 020e38e2..ec499903 100644 --- a/character/signals.py +++ b/character/signals.py @@ -1,7 +1,6 @@ # character.signals from datetime import timedelta, datetime, time -from django.db import transaction -from django.db.models.signals import pre_save, post_save, pre_delete, post_delete +from django.db.models.signals import post_save, pre_delete, post_delete from django.dispatch import receiver from django.utils import timezone @@ -41,59 +40,6 @@ def unlink_before_deletion(sender, instance, **kwargs): instance.unlink() -def recompute_character_flags(character_id: int) -> None: - """ - Recompute denormalised flags for a Character based on current links. - A character can_link if they have no active player links. - """ - if not character_id: - return - - # Character can link if they don't have any active player links - has_active_link = PlayerCharacterLink.objects.filter( - character_id=character_id, is_active=True - ).exists() - - can_link = not has_active_link - - Character.objects.filter(id=character_id).update( - can_link=can_link, - ) - - -@receiver(pre_save, sender=PlayerCharacterLink) -def link_presave_track_old_character(sender, instance, **kwargs): - """ - If someone edits a link and changes its character, we need to update BOTH: - the old character and the new character. - """ - instance._old_character_id = None - if instance.pk: - try: - old = PlayerCharacterLink.objects.only("character_id").get(pk=instance.pk) - instance._old_character_id = old.character_id - except PlayerCharacterLink.DoesNotExist: - pass - - -@receiver(post_save, sender=PlayerCharacterLink) -def link_postsave_recompute(sender, instance, **kwargs): - def _do(): - # update new/current character - recompute_character_flags(instance.character_id) - # update old character if link moved - old_id = getattr(instance, "_old_character_id", None) - if old_id and old_id != instance.character_id: - recompute_character_flags(old_id) - - transaction.on_commit(_do) - - -@receiver(post_delete, sender=PlayerCharacterLink) -def link_postdelete_recompute(sender, instance, **kwargs): - transaction.on_commit(lambda: recompute_character_flags(instance.character_id)) - - @receiver(post_delete, sender=CharacterRelationshipMembership) def delete_relationship_with_no_members(sender, instance, **kwargs): """ diff --git a/character/tests/test_filters.py b/character/tests/test_filters.py index de73f859..c90af8e1 100644 --- a/character/tests/test_filters.py +++ b/character/tests/test_filters.py @@ -17,7 +17,6 @@ def setUp(self): given_name="NPC1", birth_date=date(2000, 1, 1), sex="Male", - can_link=True, level=5, xp=100, ) @@ -25,7 +24,6 @@ def setUp(self): given_name="NPC2", birth_date=date(2000, 1, 1), sex="Female", - can_link=True, level=10, xp=500, ) @@ -45,7 +43,6 @@ def setUp(self): given_name="Player1", birth_date=date(2000, 1, 1), sex="Male", - can_link=False, level=3, xp=75, ) @@ -66,7 +63,6 @@ def setUp(self): given_name="Player2", birth_date=date(2000, 1, 1), sex="Female", - can_link=False, level=7, xp=200, ) diff --git a/character/tests/test_models.py b/character/tests/test_models.py index 45704cff..552717c4 100644 --- a/character/tests/test_models.py +++ b/character/tests/test_models.py @@ -413,13 +413,11 @@ def setUp(self): given_name="NPC1", birth_date=date(2000, 1, 1), sex="Male", - can_link=True, ) self.npc2 = Character.objects.create( given_name="NPC2", birth_date=date(2000, 1, 1), sex="Female", - can_link=True, ) # Create a player-linked character @@ -440,14 +438,10 @@ def setUp(self): given_name="Player", birth_date=date(2000, 1, 1), sex="Male", - can_link=False, ) PlayerCharacterLink.objects.create( player=self.player, character=self.player_character, is_active=True ) - # Update can_link to match real behavior - self.player_character.can_link = False - self.player_character.save() def test_is_npc_property_for_npc(self): """Test that a character without an active player link is an NPC""" @@ -483,8 +477,8 @@ def test_has_available_classmethod(self): def test_has_available_no_linkable_characters(self): """Test has_available returns False when no linkable characters exist""" - # Mark all NPCs as not linkable - Character.objects.filter(can_link=True).update(can_link=False) + # Mark all currently-linkable NPCs as reserved, so none remain linkable + Character.objects.linkable().update(is_reserved=True) self.assertFalse(Character.has_available()) def test_has_available_all_linked(self): @@ -506,65 +500,6 @@ def test_has_available_all_linked(self): self.assertFalse(Character.has_available()) -class PlayerCharacterLinkPointsTodayTests(TestCase): - """Tests for PlayerCharacterLink.player_time_today/points_today (issue #673).""" - - def setUp(self): - from progression.models import PlayerActivity - - self.PlayerActivity = PlayerActivity - self.user = user_factory(with_player=True) - self.player = self.user.player - character = Character.objects.create(given_name="Hero") - self.link = PlayerCharacterLink.objects.create( - player=self.player, character=character - ) - # Backdated well before "today" so player_time_today's max(start_of_day, - # linked_at) resolves to start_of_day in these tests, rather than to - # whatever moment setUp happened to run at. - self.link.linked_at = now() - timedelta(days=30) - self.link.save(update_fields=["linked_at"]) - - def _complete_activity(self, *, duration_seconds, completed_at): - return self.PlayerActivity.objects.create( - player=self.player, - is_complete=True, - duration=duration_seconds, - completed_at=completed_at, - ) - - def test_points_today_counts_only_activities_completed_today(self): - today_start = now().replace(hour=0, minute=0, second=0, microsecond=0) - self._complete_activity( - duration_seconds=1800, completed_at=today_start + timedelta(hours=2) - ) # 30 min today - self._complete_activity( - duration_seconds=3600, completed_at=today_start - timedelta(hours=1) - ) # 60 min yesterday - excluded - - self.assertEqual(self.link.player_time_today, 30) - self.assertEqual(self.link.points_today, 3) - - def test_points_today_excludes_activity_before_link_started(self): - today_start = now().replace(hour=0, minute=0, second=0, microsecond=0) - self.link.linked_at = today_start + timedelta(hours=5) - self.link.save(update_fields=["linked_at"]) - - self._complete_activity( - duration_seconds=1800, completed_at=today_start + timedelta(hours=1) - ) # today, but before the link started - excluded - self._complete_activity( - duration_seconds=600, completed_at=today_start + timedelta(hours=6) - ) # 10 min, after linked_at - - self.assertEqual(self.link.player_time_today, 10) - self.assertEqual(self.link.points_today, 1) - - def test_points_today_zero_with_no_activities(self): - self.assertEqual(self.link.player_time_today, 0) - self.assertEqual(self.link.points_today, 0) - - class CharacterTotalLinkPointsTests(TestCase): """Tests for Character.total_link_points (the character-side counterpart to Player.total_link_points).""" diff --git a/core/migrations/0016_gamesettings_daily_goals_completion_bonus_ap.py b/core/migrations/0016_gamesettings_daily_goals_completion_bonus_ap.py new file mode 100644 index 00000000..b30f306a --- /dev/null +++ b/core/migrations/0016_gamesettings_daily_goals_completion_bonus_ap.py @@ -0,0 +1,21 @@ +# Generated by Django 5.2.16 on 2026-08-10 00:00 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0015_featureflag"), + ] + + operations = [ + migrations.AddField( + model_name="gamesettings", + name="daily_goals_completion_bonus_ap", + field=models.IntegerField( + default=50, + help_text="Lump-sum AP awarded once per day when a player clears all three daily goals (see progression.daily_goals).", + ), + ), + ] diff --git a/core/models.py b/core/models.py index 07e9204c..da57cd33 100644 --- a/core/models.py +++ b/core/models.py @@ -90,6 +90,13 @@ class GameSettings(models.Model): max_digits=5, decimal_places=2, default="1.25" ) task_completion_xp = models.IntegerField(default=100) + daily_goals_completion_bonus_ap = models.IntegerField( + default=50, + help_text=( + "Lump-sum AP awarded once per day when a player clears all " + "three daily goals (see progression.daily_goals)." + ), + ) xp_mastery_scale = models.DecimalField( max_digits=10, decimal_places=2, @@ -162,6 +169,8 @@ def clean(self): errors["task_activity_xp_multiplier"] = "Must be > 0." if self.task_completion_xp < 0: errors["task_completion_xp"] = "Must be non-negative." + if self.daily_goals_completion_bonus_ap < 0: + errors["daily_goals_completion_bonus_ap"] = "Must be non-negative." if self.xp_mastery_scale <= 0: errors["xp_mastery_scale"] = "Must be > 0." if self.xp_mastery_multiplier_cap < 1: diff --git a/frontend/src/api/activities.ts b/frontend/src/api/activities.ts index bbb97450..2d8a8019 100644 --- a/frontend/src/api/activities.ts +++ b/frontend/src/api/activities.ts @@ -1,7 +1,18 @@ // src/api/activities.ts import type { PlayerActivity } from "../types"; +import type { OfflineActivityLogResponse } from "../types/api"; import { apiFetch } from "../utils/api"; +export interface OfflineActivityLogPayload { + /** Name for a newly-created task; ignored (and optional) when `task` is set. */ + name?: string; + description?: string; + skill?: number; + task?: number; + started_at: string; + completed_at: string; +} + export function fetchActivities(): Promise { return (async () => { const allResults: PlayerActivity[] = []; @@ -45,3 +56,12 @@ export function deleteActivity(id: number): Promise { method: "DELETE", }); } + +export function logOfflineActivity( + data: OfflineActivityLogPayload +): Promise { + return apiFetch("/player-activities/log_offline/", { + method: "POST", + body: JSON.stringify(data), + }); +} diff --git a/frontend/src/api/player.ts b/frontend/src/api/player.ts index b4263b44..ba23d15b 100644 --- a/frontend/src/api/player.ts +++ b/frontend/src/api/player.ts @@ -41,13 +41,25 @@ export const deleteAccount = async (): Promise => { return response; }; -export interface TodayPointsResponse { - // null (not 0) when the player has no active PlayerCharacterLink - see - // MeViewSet.today_points in api/views.py. Callers use this to hide the - // map view's "today" badge entirely rather than showing a zero (issue #673). - points_today: number | null; +export interface DailyGoalsState { + logged_in_today: boolean; + completed_activity_today: boolean; + activity_minutes_today: number; + minutes_goal_threshold: number; + minutes_goal_met: boolean; + all_goals_met: boolean; + bonus_awarded_today: boolean; + bonus_ap: number; } -export const fetchTodayPoints = async (): Promise => { - return apiFetch("/me/today_points/"); +export interface DailyGoalsResponse { + // null (not a set of all-false goals) when the player has no active + // PlayerCharacterLink - see MeViewSet.daily_goals in api/views.py. + // Callers use this to hide the map view's badge entirely rather than + // showing an unearned "0 of 3" (issue #673, redesigned in #751). + goals: DailyGoalsState | null; +} + +export const fetchDailyGoals = async (): Promise => { + return apiFetch("/me/daily_goals/"); }; diff --git a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss index a1f3229b..520b000d 100644 --- a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss +++ b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss @@ -14,13 +14,37 @@ flex-direction: column; } -.dateTabs { +.toolbar { display: flex; + align-items: center; gap: sp.$spacing-sm; margin-bottom: sp.$spacing-md; + width: 100%; +} + +.dateTabs { + display: flex; + gap: sp.$spacing-sm; flex-wrap: wrap; justify-content: center; - width: 100%; + flex: 1; +} + +.addOfflineButton { + @include c.apply-button-variant(primary); + flex-shrink: 0; + border: none; + border-radius: 50%; + width: 2.25rem; + height: 2.25rem; + min-width: 2.25rem; + padding: 0; + font-size: 1.25rem; + line-height: 1; + display: inline-flex; + align-items: center; + justify-content: center; + cursor: pointer; } .dateButton { diff --git a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx index 1eea100b..99bfd72f 100644 --- a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx +++ b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx @@ -2,8 +2,17 @@ import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { TooltipProvider } from "../Tooltip/Tooltip"; import ActivitiesPanel from "./ActivitiesPanel"; +function renderActivitiesPanel() { + return render( + + + + ); +} + const mockUseActivities = vi.fn(); const mockUseDeleteActivity = vi.fn(); const mockUseUpdateActivity = vi.fn(); @@ -39,7 +48,7 @@ describe("ActivitiesPanel", () => { it("renders activities and delegates edit through PlayerItemList", async () => { const user = userEvent.setup(); - render(); + renderActivitiesPanel(); expect(screen.getByText("Write docs")).toBeInTheDocument(); @@ -47,19 +56,19 @@ describe("ActivitiesPanel", () => { const input = screen.getByLabelText("activity name"); await user.clear(input); await user.type(input, "Write tests"); - await user.click(screen.getByRole("button", { name: "Save" })); + await user.tab(); await waitFor(() => { - expect(updateMutate).toHaveBeenCalledWith({ - activityId: 1, - data: { name: "Write tests" }, - }); + expect(updateMutate).toHaveBeenCalledWith( + { activityId: 1, data: { name: "Write tests" } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); }); }); it("delegates delete confirmation through PlayerItemList", async () => { const user = userEvent.setup(); - render(); + renderActivitiesPanel(); await user.click(screen.getByRole("button", { name: "Open activity Write docs" })); await user.click(within(screen.getByRole("dialog")).getByRole("button", { name: "Delete" })); diff --git a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx index 614ce3ec..72e3c820 100644 --- a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx +++ b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx @@ -6,6 +6,8 @@ import { formatDurationShort, pluralize } from "../../utils/formatUtils"; import Button from "../Button/Button"; import PlayerItemList from "../PlayerItemList/PlayerItemList"; +import Tooltip from "../Tooltip/Tooltip"; +import LogOfflineActivityModal from "../LogOfflineActivityModal/LogOfflineActivityModal"; import styles from "./ActivitiesPanel.module.scss"; type DateCategory = "today" | "yesterday" | "older"; @@ -68,6 +70,7 @@ export default function ActivitiesPanel(): React.ReactElement | null { const deleteActivity = useDeleteActivity(); const updateActivity = useUpdateActivity(); const [activeTab, setActiveTab] = useState("today"); + const [isLogModalOpen, setIsLogModalOpen] = useState(false); const bucketed = useMemo(() => bucketActivities(activities ?? []), [activities]); @@ -103,11 +106,14 @@ export default function ActivitiesPanel(): React.ReactElement | null { const hasTabActivities = groupedByCategory[activeTab].length > 0; const handleEdit = useCallback( - (activity: PlayerActivity, name: string) => { - updateActivity.mutate({ - activityId: activity.id, - data: { name }, - }); + (activity: PlayerActivity, name: string, callbacks?: { onSuccess?: () => void; onError?: () => void }) => { + updateActivity.mutate( + { + activityId: activity.id, + data: { name }, + }, + callbacks, + ); }, [updateActivity], ); @@ -124,8 +130,8 @@ export default function ActivitiesPanel(): React.ReactElement | null { return (
- {hasActivities && ( - <> +
+ {hasActivities && (
{dateTabs.map(({ key, label }) => (
+ )} + + + +
+ {isLogModalOpen && ( + setIsLogModalOpen(false)} /> + )} + + {hasActivities && ( + <> {hasTabActivities ? (
{Object.entries(activitiesByDay).map(([dateKey, dayActivities]) => { diff --git a/frontend/src/components/ActivityInput/ActivityInput.module.scss b/frontend/src/components/ActivityInput/ActivityInput.module.scss index 6a5a3d63..f4725474 100644 --- a/frontend/src/components/ActivityInput/ActivityInput.module.scss +++ b/frontend/src/components/ActivityInput/ActivityInput.module.scss @@ -4,7 +4,7 @@ @use '../../styles/semantic/spacing' as sp; @use 'sass:map'; -$row-control-height: 44px; +$row-control-height: sp.$form-control-height; .control { height: $row-control-height; display: flex; @@ -216,14 +216,6 @@ $row-control-height: 44px; } @include m.respond-to(md, down) { - .control { - height: 40px; - } - - .grow { - min-height: 40px; - } - .inputText { font-size: 1rem; line-height: 1.1; @@ -242,14 +234,6 @@ $row-control-height: 44px; } @include m.respond-to(sm, down) { - .control { - height: 38px; - } - - .grow { - min-height: 38px; - } - .inputText { font-size: 0.9rem; line-height: 1.05; diff --git a/frontend/src/components/ActivityInput/ActivityInput.tsx b/frontend/src/components/ActivityInput/ActivityInput.tsx index 89e77ae7..afb1eb4b 100644 --- a/frontend/src/components/ActivityInput/ActivityInput.tsx +++ b/frontend/src/components/ActivityInput/ActivityInput.tsx @@ -7,6 +7,7 @@ import EntitySearchInput from "../EntitySearchInput/EntitySearchInput"; import styles from "./ActivityInput.module.scss"; import { useActivityInput } from "./useActivityInput"; import SupportFlowModal from "../SupportFlow/SupportFlowModal"; +import { formatDuration } from "../../utils/formatUtils"; export default function ActivityInput() { const { @@ -14,8 +15,7 @@ export default function ActivityInput() { setName, isActive, inputValue, - minutes, - seconds, + elapsed, formattedLimit, showAutoStopWarning, flowState, @@ -63,7 +63,7 @@ export default function ActivityInput() {
- {minutes}:{seconds.toString().padStart(2, "0")} + {formatDuration(elapsed)}
+ +
+
+ + ); + } + + return ( + +
+
+ + Task * + + + {fieldErrors.task && ( +

{fieldErrors.task}

+ )} +
+ +
+ + Duration * + +
+ setDurationHours(value as string)} + className={styles.durationPartField} + /> + setDurationMinutes((value as string).slice(0, 2))} + maxLength={2} + className={styles.durationPartField} + /> +
+ {fieldErrors.duration && ( +

{fieldErrors.duration}

+ )} +
+ +
+ Completed +
+ handleCompletionDateChange(value as string)} + max={maxCompletionDate} + /> + handleCompletionTimeChange(value as string)} + /> +
+ {fieldErrors.completedAt && ( +

{fieldErrors.completedAt}

+ )} +
+ + {!Number.isNaN(totalDurationMinutes) && totalDurationMinutes > 0 && ( +

+ Logging {formatDurationShort(Math.round(totalDurationMinutes * 60))} +

+ )} + + {eligibility && ( +

+ {eligibility.message} +

+ )} + + {submitError && ( +

{submitError}

+ )} + +
+ + +
+
+
+ ); +} diff --git a/frontend/src/components/LogOfflineActivityModal/useLogOfflineActivityForm.ts b/frontend/src/components/LogOfflineActivityModal/useLogOfflineActivityForm.ts new file mode 100644 index 00000000..03d24d1d --- /dev/null +++ b/frontend/src/components/LogOfflineActivityModal/useLogOfflineActivityForm.ts @@ -0,0 +1,242 @@ +import { useCallback, useMemo, useState } from "react"; + +import { useLogOfflineActivity } from "../../hooks/useActivities"; +import { useGame } from "../../hooks/useGame"; +import type { SearchEntity } from "../EntitySearchInput/useEntitySearchInput"; +import type { OfflineActivityLogResponse } from "../../types"; + +// Must match OFFLINE_XP_ELIGIBLE_BACKDATE_WINDOW in progression/services.py — +// duplicated here only for the client-side "will this earn XP" hint; the +// backend remains the authority on whether XP is actually awarded. +const XP_ELIGIBLE_BACKDATE_DAYS = 7; + +function pad(n: number): string { + return String(n).padStart(2, "0"); +} + +function todayDateValue(): string { + const now = new Date(); + return `${now.getFullYear()}-${pad(now.getMonth() + 1)}-${pad(now.getDate())}`; +} + +function nowTimeValue(): string { + const now = new Date(); + return `${pad(now.getHours())}:${pad(now.getMinutes())}`; +} + +function extractErrorMessage(error: unknown): string { + if (error instanceof Error && error.message) { + try { + const parsed = JSON.parse(error.message) as Record; + if (typeof parsed.message === "string") return parsed.message; + + const firstFieldError = Object.values(parsed).find( + (value): value is string[] => + Array.isArray(value) && typeof value[0] === "string" + ); + if (firstFieldError) return firstFieldError[0]; + } catch { + // Not a JSON error body — fall back to the raw message below. + } + return error.message; + } + return "Something went wrong logging this activity. Please try again."; +} + +export interface FieldErrors { + task?: string; + duration?: string; + completedAt?: string; +} + +export function useLogOfflineActivityForm(onLogged?: () => void) { + const { player, fetchPlayerAndCharacter } = useGame(); + const isPremium = Boolean(player?.is_premium); + const logOfflineActivity = useLogOfflineActivity(); + + const [taskName, setTaskName] = useState(""); + const [selectedTaskId, setSelectedTaskId] = useState(null); + const [durationHours, setDurationHours] = useState(""); + const [durationMinutes, setDurationMinutes] = useState(""); + const [completionDate, setCompletionDate] = useState(todayDateValue); + const [completionTime, setCompletionTime] = useState(nowTimeValue); + // Fixed at mount: caps the native date picker so future dates can't be + // selected at all, rather than allowing them and erroring afterwards. + const [maxCompletionDate] = useState(todayDateValue); + const [fieldErrors, setFieldErrors] = useState({}); + const [submitError, setSubmitError] = useState(null); + const [result, setResult] = useState(null); + // Read once at mount (a lazy initializer, not a render-phase call) and + // refreshed by the date/time change handlers below — those are event + // handlers, where reading the clock is fine. + const [nowMs, setNowMs] = useState(() => Date.now()); + + const handleCompletionDateChange = useCallback((value: string) => { + setCompletionDate(value); + setNowMs(Date.now()); + }, []); + + const handleCompletionTimeChange = useCallback((value: string) => { + setCompletionTime(value); + setNowMs(Date.now()); + }, []); + + const handleTaskNameChange = useCallback((value: string) => { + setTaskName(value); + setSelectedTaskId(null); + }, []); + + const handleTaskSelect = useCallback((entity: SearchEntity) => { + setTaskName(entity.name); + setSelectedTaskId(typeof entity.id === "number" ? entity.id : Number(entity.id)); + }, []); + + const totalDurationMinutes = useMemo(() => { + const hours = Number(durationHours || 0); + const minutes = Number(durationMinutes || 0); + if (Number.isNaN(hours) || Number.isNaN(minutes)) return NaN; + return hours * 60 + minutes; + }, [durationHours, durationMinutes]); + + const completedAt = useMemo(() => { + if (!completionDate || !completionTime) return null; + const date = new Date(`${completionDate}T${completionTime}`); + return Number.isNaN(date.getTime()) ? null : date; + }, [completionDate, completionTime]); + + const eligibility = useMemo(() => { + if (!completedAt) return null; + + if (!isPremium) { + return { + tone: "info" as const, + message: "This will be recorded, but offline activities only earn XP for Premium accounts.", + }; + } + + const ageDays = (nowMs - completedAt.getTime()) / (24 * 60 * 60 * 1000); + if (ageDays > XP_ELIGIBLE_BACKDATE_DAYS) { + return { + tone: "warning" as const, + message: `This is more than ${XP_ELIGIBLE_BACKDATE_DAYS} days old, so it will be recorded but won't earn XP.`, + }; + } + + return { + tone: "success" as const, + message: "This activity is eligible for XP, subject to your daily logging limits.", + }; + }, [completedAt, isPremium, nowMs]); + + const validate = useCallback((): FieldErrors => { + const errors: FieldErrors = {}; + + if (!taskName.trim()) { + errors.task = "Enter or select a task."; + } + + const hours = durationHours ? Number(durationHours) : 0; + const minutes = durationMinutes ? Number(durationMinutes) : 0; + if (Number.isNaN(hours) || hours < 0) { + errors.duration = "Hours must be a positive number."; + } else if (Number.isNaN(minutes) || minutes < 0 || minutes > 59) { + errors.duration = "Minutes must be between 0 and 59."; + } else if (totalDurationMinutes <= 0) { + errors.duration = "Enter a duration greater than zero."; + } + + if (!completionDate || !completionTime) { + errors.completedAt = "Enter a completion date and time."; + } else if (!completedAt) { + errors.completedAt = "Enter a valid completion date and time."; + } else if (completedAt.getTime() > Date.now()) { + errors.completedAt = "Completion date/time can't be in the future."; + } + + return errors; + }, [ + completedAt, + completionDate, + completionTime, + durationHours, + durationMinutes, + taskName, + totalDurationMinutes, + ]); + + const reset = useCallback(() => { + setTaskName(""); + setSelectedTaskId(null); + setDurationHours(""); + setDurationMinutes(""); + setCompletionDate(todayDateValue()); + setCompletionTime(nowTimeValue()); + setNowMs(Date.now()); + setFieldErrors({}); + setSubmitError(null); + setResult(null); + }, []); + + const handleSubmit = useCallback( + (event: React.FormEvent) => { + event.preventDefault(); + setSubmitError(null); + + const errors = validate(); + setFieldErrors(errors); + if (Object.keys(errors).length > 0 || !completedAt) return; + + const durationSeconds = Math.round(totalDurationMinutes * 60); + const startedAt = new Date(completedAt.getTime() - durationSeconds * 1000); + + logOfflineActivity.mutate( + { + ...(selectedTaskId ? { task: selectedTaskId } : { name: taskName.trim() }), + started_at: startedAt.toISOString(), + completed_at: completedAt.toISOString(), + }, + { + onSuccess: (data) => { + setResult(data); + if (data.xp_gained > 0) fetchPlayerAndCharacter(); + onLogged?.(); + }, + onError: (error) => setSubmitError(extractErrorMessage(error)), + } + ); + }, + [ + completedAt, + fetchPlayerAndCharacter, + logOfflineActivity, + onLogged, + selectedTaskId, + taskName, + totalDurationMinutes, + validate, + ] + ); + + return { + taskName, + handleTaskNameChange, + handleTaskSelect, + durationHours, + setDurationHours, + durationMinutes, + setDurationMinutes, + totalDurationMinutes, + completionDate, + handleCompletionDateChange, + completionTime, + handleCompletionTimeChange, + maxCompletionDate, + fieldErrors, + submitError, + result, + eligibility, + isSubmitting: logOfflineActivity.isPending, + handleSubmit, + reset, + }; +} diff --git a/frontend/src/components/Map/Map.test.tsx b/frontend/src/components/Map/Map.test.tsx index 33f39f40..c17e6d9c 100644 --- a/frontend/src/components/Map/Map.test.tsx +++ b/frontend/src/components/Map/Map.test.tsx @@ -1165,7 +1165,6 @@ describe('PopulationCentreMap entity detail card', () => { const dialog = await screen.findByRole('dialog', { name: 'House 2' }); expect(dialog).toHaveTextContent('1 / 4'); expect(dialog).toHaveTextContent('Alice'); - expect(dialog).toHaveTextContent('idle'); }); it('switches to a resident\'s own detail card when clicked inside the building detail card', async () => { diff --git a/frontend/src/components/Modal/Modal.module.scss b/frontend/src/components/Modal/Modal.module.scss index 0ca97f77..6022ecb9 100644 --- a/frontend/src/components/Modal/Modal.module.scss +++ b/frontend/src/components/Modal/Modal.module.scss @@ -53,8 +53,7 @@ overflow-y: auto; overflow-x: hidden; min-height: 0; - padding-top: sp.$spacing-md; - padding-bottom: sp.$spacing-md; + padding: sp.$spacing-md sp.$spacing-sm; @include tc.two-column-layout; justify-content: flex-start; diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.module.scss b/frontend/src/components/PlayerItemList/PlayerItemList.module.scss index 87e9fd30..d577afcd 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.module.scss +++ b/frontend/src/components/PlayerItemList/PlayerItemList.module.scss @@ -156,19 +156,12 @@ } } -.childList { - list-style: none; - margin: sp.$spacing-xs 0 0; - padding: 0 0 0 sp.$spacing-lg; - display: flex; - flex-direction: column; - gap: sp.$spacing-xs; -} - .childItem { - background: transparent; - border-color: rgba(c.$color-border-primary, 0.12); - opacity: 0.9; + // Shrink width by the indent instead of just shifting it, so the right + // edge still lines up with the parent's — .listItem's width: 100% would + // otherwise push the box that far past the parent's right edge. + width: calc(100% - #{sp.$spacing-lg}); + margin-left: sp.$spacing-lg; } .itemCompleted { @@ -235,9 +228,10 @@ .editInput { flex: 1; min-width: 0; + height: sp.$form-control-height; padding: sp.$spacing-xs sp.$spacing-sm; border: 2px solid rgba(c.$color-status-success, 0.45); - border-radius: sp.$border-radius; + border-radius: sp.$form-control-radius; font-size: 1em; font-family: inherit; transition: border-color 0.2s ease; @@ -259,6 +253,7 @@ } .editConfirmActions { + position: relative; display: flex; justify-content: flex-start; align-items: center; @@ -290,3 +285,45 @@ color: rgba(0, 0, 0, 0.65); @include t.apply-text-style(t.$text-caption); } + +.saveStatus { + position: absolute; + right: 0; + top: 50%; + transform: translateY(-50%); + padding: sp.$spacing-xs sp.$spacing-sm; + border-radius: sp.$border-radius; + border: 1px solid rgba(c.$color-border-primary, 0.4); + background: rgba(c.$color-bg, 0.95); + box-shadow: 0 2px 8px rgba(0, 0, 0, 0.25); + pointer-events: none; + z-index: 1; + font-weight: 600; + @include t.apply-text-style(t.$text-caption); + animation: saveStatusFadeIn 0.15s ease; +} + +.saveStatus-saving { + color: rgba(0, 0, 0, 0.6); +} + +.saveStatus-saved { + color: c.$color-status-success; + animation: saveStatusFadeInOut #{2500ms} ease forwards; +} + +.saveStatus-error { + color: c.$color-status-danger; +} + +@keyframes saveStatusFadeIn { + from { opacity: 0; } + to { opacity: 1; } +} + +@keyframes saveStatusFadeInOut { + 0% { opacity: 0; } + 10% { opacity: 1; } + 80% { opacity: 1; } + 100% { opacity: 0; } +} diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx b/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx index 682abdd2..6142c2df 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx +++ b/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx @@ -26,7 +26,7 @@ describe("PlayerItemList", () => { expect(screen.getByText("Duration: 15m • 15 XP gained")).toBeInTheDocument(); }); - it("opens the edit modal and saves a trimmed name", async () => { + it("autosaves a trimmed name on blur", async () => { const user = userEvent.setup(); const onEdit = vi.fn(); @@ -43,10 +43,57 @@ describe("PlayerItemList", () => { const input = screen.getByLabelText("activity name"); await user.clear(input); await user.type(input, " Deep work "); - await user.click(screen.getByRole("button", { name: "Save" })); + await user.tab(); - expect(onEdit).toHaveBeenCalledWith(items[0], "Deep work"); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(onEdit).toHaveBeenCalledWith( + items[0], + "Deep work", + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); + // Autosave doesn't close the modal — there's no explicit Save action anymore. + expect(screen.getByRole("dialog")).toBeInTheDocument(); + }); + + it("does not re-fire the save when blurring without changing the name", async () => { + const user = userEvent.setup(); + const onEdit = vi.fn(); + + render( + , + ); + + await user.click(screen.getByRole("button", { name: "Open activity Write docs" })); + const input = screen.getByLabelText("activity name"); + await user.click(input); + await user.tab(); + + expect(onEdit).not.toHaveBeenCalled(); + }); + + it("shows a Saved indicator once the autosave succeeds, then shows a warning on failure", async () => { + const user = userEvent.setup(); + const onEdit = vi.fn((_item, _name, callbacks) => callbacks?.onSuccess?.()); + + const { rerender } = render( + , + ); + + await user.click(screen.getByRole("button", { name: "Open activity Write docs" })); + const input = screen.getByLabelText("activity name"); + await user.clear(input); + await user.type(input, "Deep work"); + await user.tab(); + + expect(await screen.findByText("Saved")).toBeInTheDocument(); + + const failingOnEdit = vi.fn((_item, _name, callbacks) => callbacks?.onError?.()); + rerender(); + + await user.clear(input); + await user.type(input, "Deeper work"); + await user.tab(); + + expect(await screen.findByText("Couldn't save — try again")).toBeInTheDocument(); }); it("opens the delete modal and confirms deletion", async () => { @@ -154,7 +201,7 @@ describe("PlayerItemList", () => { const child = { id: 11, name: "Child task" }; const flatItems = [parent, child]; - it("renders children nested under their parent without a top-level row", () => { + it("renders children as independent rows, indented, directly after their parent", () => { render( { expect(screen.getByRole("button", { name: "Open task Parent task" })).toBeInTheDocument(); expect(screen.getByRole("button", { name: "Open task Child task" })).toBeInTheDocument(); - // The child is not rendered as its own top-level list item. + // The child is not duplicated as a second top-level entry sourced from `items`. expect(screen.getAllByRole("button", { name: /^Open task / })).toHaveLength(2); + + // Both render as siblings within the list, not one nested inside the other. + const rows = screen.getAllByRole("listitem"); + expect(rows).toHaveLength(2); + expect(rows[0]).toHaveTextContent("Parent task"); + expect(rows[1]).toHaveTextContent("Child task"); + expect(within(rows[0]).queryByText("Child task")).not.toBeInTheDocument(); }); it("is unaffected when getChildren is not passed (ProjectsPanel-style usage)", () => { diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.tsx b/frontend/src/components/PlayerItemList/PlayerItemList.tsx index 53356cc3..9d0f2e9c 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.tsx +++ b/frontend/src/components/PlayerItemList/PlayerItemList.tsx @@ -3,10 +3,12 @@ import classNames from "classnames"; import Button from "../Button/Button"; import List from "../List/List"; -import Li from "../List/Li"; import Modal from "../Modal/Modal"; +import SaveStatusIndicator from "./SaveStatusIndicator"; import { usePlayerItemListControls } from "./usePlayerItemListControls"; import { usePlayerItemModal } from "./usePlayerItemModal"; +import type { SaveCallbacks } from "./usePlayerItemModal"; +import type { SaveStatusHelpers } from "./useSaveStatus"; import styles from "./PlayerItemList.module.scss"; export interface SortOption { @@ -30,8 +32,8 @@ interface PlayerItemListProps onToggleComplete?: (item: T) => void; getItemKey?: (item: T, index: number) => string | number; renderItemMeta?: (item: T) => React.ReactNode; - renderEditSummary?: (item: T) => React.ReactNode; - onEdit?: (item: T, name: string) => void; + renderEditSummary?: (item: T, saveHelpers: SaveStatusHelpers) => React.ReactNode; + onEdit?: (item: T, name: string, callbacks?: SaveCallbacks) => void; onDelete?: (item: T) => void; hoverEdit?: boolean; renderRowActions?: (item: T) => React.ReactNode; @@ -92,6 +94,7 @@ export default function PlayerItemList { + // Sort/filter controls only apply to top-level items; a child keeps its + // place directly after its parent (in `getChildren`'s order) rather than + // being reordered independently. + const flatDisplayItems = useMemo(() => { if (!getChildren) return displayItems; - return displayItems.filter( + const topLevel = displayItems.filter( (item) => item.id === undefined || !childIds.has(item.id) ); + return topLevel.flatMap((item) => [item, ...(getChildren(item) ?? [])]); }, [displayItems, getChildren, childIds]); const renderRow = (item: T): React.ReactNode => ( @@ -252,7 +259,7 @@ export default function PlayerItemList classNames(styles.item, { + [styles.childItem]: item.id !== undefined && childIds.has(item.id), [styles.itemCompleted]: isItemComplete?.(item), }) } - renderItem={(item) => { - const children = getChildren?.(item); - return ( - <> - {renderRow(item)} - {children?.length ? ( -
    - {children.map((child, index) => ( -
  • - {renderRow(child)} -
  • - ))} -
- ) : null} - - ); - }} + renderItem={(item) => renderRow(item)} /> @@ -337,6 +324,7 @@ export default function PlayerItemList setEditingName(event.target.value)} + onBlur={handleEditSave} autoFocus onKeyDown={(event) => { if (event.key === "Enter") handleEditSave(); @@ -350,19 +338,15 @@ export default function PlayerItemList{modalSummary} ) : null}
- {canEdit ? ( - - ) : null} {canDelete ? ( ) : null} +
)} diff --git a/frontend/src/components/PlayerItemList/SaveStatusIndicator.tsx b/frontend/src/components/PlayerItemList/SaveStatusIndicator.tsx new file mode 100644 index 00000000..abe08832 --- /dev/null +++ b/frontend/src/components/PlayerItemList/SaveStatusIndicator.tsx @@ -0,0 +1,27 @@ +import classNames from "classnames"; +import type { SaveStatus } from "./useSaveStatus"; +import styles from "./PlayerItemList.module.scss"; + +interface SaveStatusIndicatorProps { + status: SaveStatus; +} + +const STATUS_TEXT: Record, string> = { + saving: "Saving…", + saved: "Saved", + error: "Couldn't save — try again", +}; + +export default function SaveStatusIndicator({ status }: SaveStatusIndicatorProps) { + if (status === "idle") return null; + + return ( +
+ {STATUS_TEXT[status]} +
+ ); +} diff --git a/frontend/src/components/PlayerItemList/usePlayerItemModal.ts b/frontend/src/components/PlayerItemList/usePlayerItemModal.ts index 8884a21f..4e552cda 100644 --- a/frontend/src/components/PlayerItemList/usePlayerItemModal.ts +++ b/frontend/src/components/PlayerItemList/usePlayerItemModal.ts @@ -1,11 +1,18 @@ -import { useCallback, useMemo, useState } from "react"; +import { useCallback, useMemo, useRef, useState } from "react"; + +import { useSaveStatus, type SaveStatusHelpers } from "./useSaveStatus"; + +export interface SaveCallbacks { + onSuccess?: () => void; + onError?: () => void; +} interface UsePlayerItemModalProps { items: T[]; getItemName: (item: T) => string; renderItemMeta?: (item: T) => React.ReactNode; - renderEditSummary?: (item: T) => React.ReactNode; - onEdit?: (item: T, name: string) => void; + renderEditSummary?: (item: T, saveHelpers: SaveStatusHelpers) => React.ReactNode; + onEdit?: (item: T, name: string, callbacks?: SaveCallbacks) => void; onDelete?: (item: T) => void; } @@ -20,34 +27,55 @@ export function usePlayerItemModal(null); const [editingName, setEditingName] = useState(""); const [confirmingDelete, setConfirmingDelete] = useState(false); + // Tracks the last name we've committed (or started committing) a save + // for, so blur/Enter only fire a request when the name actually changed. + const lastSavedNameRef = useRef(""); + + const { saveStatus, reportSaving, reportSaved, reportError, resetSaveStatus } = useSaveStatus(); const handleOpenItem = useCallback( (item: T) => { setActiveItem(item); - setEditingName(getItemName(item)); + const name = getItemName(item); + setEditingName(name); + lastSavedNameRef.current = name; + resetSaveStatus(); }, - [getItemName], + [getItemName, resetSaveStatus], ); const handleModalClose = useCallback(() => { setActiveItem(null); setEditingName(""); setConfirmingDelete(false); - }, []); + resetSaveStatus(); + }, [resetSaveStatus]); - const handleEditSave = useCallback(() => { + // Autosaves the name field. Fires on blur (and Enter) rather than on + // every keystroke, mirroring the due-date field's commit-on-blur pattern. + const commitNameEdit = useCallback(() => { if (!activeItem || !onEdit) { return; } const trimmedName = editingName.trim(); - if (!trimmedName) { + if (!trimmedName || trimmedName === lastSavedNameRef.current) { return; } - onEdit(activeItem, trimmedName); - handleModalClose(); - }, [activeItem, editingName, onEdit, handleModalClose]); + lastSavedNameRef.current = trimmedName; + reportSaving(); + onEdit(activeItem, trimmedName, { + onSuccess: reportSaved, + onError: reportError, + }); + }, [activeItem, editingName, onEdit, reportSaving, reportSaved, reportError]); + + // handleEditSave is kept as the exported name for the commit action + // (bound to the name input's blur/Enter handlers) since it's still the + // thing that saves the edit — there's just no explicit "Save" button + // triggering it anymore. + const handleEditSave = commitNameEdit; const handleDeleteRequest = useCallback(() => { setConfirmingDelete(true); @@ -82,23 +110,29 @@ export function usePlayerItemModal( + () => ({ reportSaving, reportSaved, reportError }), + [reportSaving, reportSaved, reportError], + ); + const modalSummary = useMemo(() => { if (!liveActiveItem) { return null; } return ( - renderEditSummary?.(liveActiveItem) ?? + renderEditSummary?.(liveActiveItem, saveHelpers) ?? renderItemMeta?.(liveActiveItem) ?? null ); - }, [liveActiveItem, renderEditSummary, renderItemMeta]); + }, [liveActiveItem, renderEditSummary, renderItemMeta, saveHelpers]); return { activeItem, liveActiveItem, editingName, confirmingDelete, + saveStatus, activeItemName, modalSummary, diff --git a/frontend/src/components/PlayerItemList/useSaveStatus.ts b/frontend/src/components/PlayerItemList/useSaveStatus.ts new file mode 100644 index 00000000..077f03da --- /dev/null +++ b/frontend/src/components/PlayerItemList/useSaveStatus.ts @@ -0,0 +1,77 @@ +import { useCallback, useEffect, useRef, useState } from "react"; + +export type SaveStatus = "idle" | "saving" | "saved" | "error"; + +export interface SaveStatusHelpers { + reportSaving: () => void; + reportSaved: () => void; + reportError: () => void; +} + +const SAVED_DISPLAY_MS = 2500; +// Saves usually resolve fast enough that "Saving…" would just flash on +// screen; only show it once a save has been pending this long. +const SAVING_DISPLAY_DELAY_MS = 150; + +/** + * Tracks a single in-flight autosave's status so a modal can show a brief + * "Saved"/"Couldn't save" indicator. "saved" auto-clears after a few + * seconds; "error" persists until the next save attempt or a manual reset, + * since a missed save is worse than a lingering banner. + */ +export function useSaveStatus() { + const [saveStatus, setSaveStatus] = useState("idle"); + const hideTimerRef = useRef | null>(null); + const savingTimerRef = useRef | null>(null); + + const clearHideTimer = useCallback(() => { + if (hideTimerRef.current) { + clearTimeout(hideTimerRef.current); + hideTimerRef.current = null; + } + }, []); + + const clearSavingTimer = useCallback(() => { + if (savingTimerRef.current) { + clearTimeout(savingTimerRef.current); + savingTimerRef.current = null; + } + }, []); + + const reportSaving = useCallback(() => { + clearHideTimer(); + clearSavingTimer(); + savingTimerRef.current = setTimeout(() => { + savingTimerRef.current = null; + setSaveStatus("saving"); + }, SAVING_DISPLAY_DELAY_MS); + }, [clearHideTimer, clearSavingTimer]); + + const reportSaved = useCallback(() => { + clearHideTimer(); + clearSavingTimer(); + setSaveStatus("saved"); + hideTimerRef.current = setTimeout(() => setSaveStatus("idle"), SAVED_DISPLAY_MS); + }, [clearHideTimer, clearSavingTimer]); + + const reportError = useCallback(() => { + clearHideTimer(); + clearSavingTimer(); + setSaveStatus("error"); + }, [clearHideTimer, clearSavingTimer]); + + const resetSaveStatus = useCallback(() => { + clearHideTimer(); + clearSavingTimer(); + setSaveStatus("idle"); + }, [clearHideTimer, clearSavingTimer]); + + useEffect(() => { + return () => { + clearHideTimer(); + clearSavingTimer(); + }; + }, [clearHideTimer, clearSavingTimer]); + + return { saveStatus, reportSaving, reportSaved, reportError, resetSaveStatus }; +} diff --git a/frontend/src/components/ProjectsPanel/ProjectsPanel.test.tsx b/frontend/src/components/ProjectsPanel/ProjectsPanel.test.tsx index 2adb9039..be0a916f 100644 --- a/frontend/src/components/ProjectsPanel/ProjectsPanel.test.tsx +++ b/frontend/src/components/ProjectsPanel/ProjectsPanel.test.tsx @@ -77,16 +77,16 @@ describe("ProjectsPanel", () => { const input = screen.getByLabelText("project name"); await user.clear(input); await user.type(input, "Platform refresh"); - await user.click(screen.getByRole("button", { name: "Save" })); + await user.tab(); await waitFor(() => { - expect(updateMutate).toHaveBeenCalledWith({ - id: 1, - data: { name: "Platform refresh" }, - }); + expect(updateMutate).toHaveBeenCalledWith( + { id: 1, data: { name: "Platform refresh" } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); }); - await user.click(screen.getByRole("button", { name: "Open project Website overhaul" })); + // The modal stays open after an autosave — no explicit Save click to close it. await user.click(within(screen.getByRole("dialog")).getByRole("button", { name: "Delete" })); await user.click(within(screen.getByRole("dialog")).getByRole("button", { name: "Delete" })); diff --git a/frontend/src/components/SkillsPanel/SkillsPanel.test.tsx b/frontend/src/components/SkillsPanel/SkillsPanel.test.tsx index 50d3b3ec..a927c541 100644 --- a/frontend/src/components/SkillsPanel/SkillsPanel.test.tsx +++ b/frontend/src/components/SkillsPanel/SkillsPanel.test.tsx @@ -44,13 +44,13 @@ describe("SkillsPanel", () => { const input = screen.getByLabelText("skill name"); await user.clear(input); await user.type(input, "Research"); - await user.click(screen.getByRole("button", { name: "Save" })); + await user.tab(); await waitFor(() => { - expect(updateMutate).toHaveBeenCalledWith({ - id: 1, - data: { name: "Research" }, - }); + expect(updateMutate).toHaveBeenCalledWith( + { id: 1, data: { name: "Research" } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); }); }); diff --git a/frontend/src/components/TasksPanel/TasksPanel.test.tsx b/frontend/src/components/TasksPanel/TasksPanel.test.tsx index 5671697c..6ddb5e8f 100644 --- a/frontend/src/components/TasksPanel/TasksPanel.test.tsx +++ b/frontend/src/components/TasksPanel/TasksPanel.test.tsx @@ -284,13 +284,13 @@ describe("TasksPanel", () => { const input = screen.getByLabelText("task name"); await user.clear(input); await user.type(input, "Evening routine"); - await user.click(within(screen.getByRole("dialog")).getByRole("button", { name: "Save" })); + await user.tab(); await waitFor(() => { - expect(updateMutate).toHaveBeenCalledWith({ - id: 1, - data: { name: "Evening routine" }, - }); + expect(updateMutate).toHaveBeenCalledWith( + { id: 1, data: { name: "Evening routine" } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); }); }); @@ -325,7 +325,7 @@ describe("TasksPanel", () => { total_records: 0, }; - it("renders a subtask nested under its parent", () => { + it("renders a subtask as an independent, indented row directly after its parent", () => { mockUseTasks.mockReturnValue({ isLoading: false, data: [parentTask, childTask], @@ -336,9 +336,14 @@ describe("TasksPanel", () => { const childButton = screen.getAllByRole("button", { name: "Edit task Child subtask" })[0]; expect(parentButton).toBeInTheDocument(); expect(childButton).toBeInTheDocument(); - // The subtask is nested inside the parent's
  • , not a sibling top-level row. + // The subtask is its own sibling row, not nested inside the parent's
  • . const parentListItem = parentButton.closest("li"); - expect(parentListItem).toContainElement(childButton); + const childListItem = childButton.closest("li"); + expect(parentListItem).not.toBe(childListItem); + expect(parentListItem).not.toContainElement(childButton); + + const rows = screen.getAllByRole("listitem"); + expect(rows.indexOf(childListItem!)).toBe(rows.indexOf(parentListItem!) + 1); }); it("hides a completed parent and its subtasks together", () => { @@ -405,10 +410,10 @@ describe("TasksPanel", () => { await user.tab(); await waitFor(() => { - expect(updateMutate).toHaveBeenCalledWith({ - id: 1, - data: { due_at: expect.any(String) }, - }); + expect(updateMutate).toHaveBeenCalledWith( + { id: 1, data: { due_at: expect.any(String) } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); }); }); }); diff --git a/frontend/src/components/TasksPanel/TasksPanel.tsx b/frontend/src/components/TasksPanel/TasksPanel.tsx index e8d64ca0..3da46e7a 100644 --- a/frontend/src/components/TasksPanel/TasksPanel.tsx +++ b/frontend/src/components/TasksPanel/TasksPanel.tsx @@ -107,7 +107,7 @@ export default function TasksPanel({ ); }} - renderEditSummary={(taskItem) => { + renderEditSummary={(taskItem, saveHelpers) => { const summary = getTaskEditSummary(taskItem); const hasSubtasks = (taskItem.subtask_count ?? 0) > 0; const parentOptions = topLevelTasks.filter((t) => t.id !== taskItem.id); @@ -162,58 +162,68 @@ export default function TasksPanel({ type="datetime-local" className={styles.dueDateInput} defaultValue={toDatetimeLocalValue(taskItem.due_at)} - onBlur={(event) => - updateTask.mutate({ - id: taskItem.id, - data: { due_at: fromDatetimeLocalValue(event.target.value) }, - }) - } + onBlur={(event) => { + saveHelpers.reportSaving(); + updateTask.mutate( + { + id: taskItem.id, + data: { due_at: fromDatetimeLocalValue(event.target.value) }, + }, + { onSuccess: saveHelpers.reportSaved, onError: saveHelpers.reportError }, + ); + }} /> {taskItem.due_at && ( )} - {taskItem.parent == null && ( -
    - - - { + saveHelpers.reportSaving(); + updateTask.mutate( + { id: taskItem.id, data: { parent: event.target.value ? Number(event.target.value) : null }, - }) - } - > - - {parentOptions.map((option) => ( - - ))} - - -
    - )} + }, + { onSuccess: saveHelpers.reportSaved, onError: saveHelpers.reportError }, + ); + }} + > + + {parentOptions.map((option) => ( + + ))} + + + ); }} diff --git a/frontend/src/components/TasksPanel/useTasksPanel.tsx b/frontend/src/components/TasksPanel/useTasksPanel.tsx index 9da699f5..af963a66 100644 --- a/frontend/src/components/TasksPanel/useTasksPanel.tsx +++ b/frontend/src/components/TasksPanel/useTasksPanel.tsx @@ -245,15 +245,8 @@ export function useTasksPanel(openTaskId?: number | null, onOpenNote?: (noteId: }, []); const handleEdit = useCallback( - (task: ItemRecord, name: string, options?: { parent?: number | null; due_at?: string | null }) => { - updateTask.mutate({ - id: task.id, - data: { - name, - ...(options?.parent !== undefined ? { parent: options.parent } : {}), - ...(options?.due_at !== undefined ? { due_at: options.due_at } : {}), - }, - }); + (task: ItemRecord, name: string, callbacks?: { onSuccess?: () => void; onError?: () => void }) => { + updateTask.mutate({ id: task.id, data: { name } }, callbacks); }, [updateTask] ); diff --git a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.module.scss b/frontend/src/components/TodayPointsBadge/TodayPointsBadge.module.scss deleted file mode 100644 index 905ae3fe..00000000 --- a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.module.scss +++ /dev/null @@ -1,12 +0,0 @@ -@use '../../styles/semantic/colors' as c; -@use '../../styles/semantic/typography' as t; -@use '../../styles/semantic/spacing' as sp; - -.badge { - background: c.$color-bg; - border: 1px solid c.$color-border-primary; - border-radius: sp.$border-radius; - padding: 0.25rem 0.6rem; - white-space: nowrap; - @include t.apply-text-style(t.$text-body); -} diff --git a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.test.tsx b/frontend/src/components/TodayPointsBadge/TodayPointsBadge.test.tsx deleted file mode 100644 index c2c50430..00000000 --- a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.test.tsx +++ /dev/null @@ -1,46 +0,0 @@ -import { render, screen } from '@testing-library/react'; -import { describe, expect, it, vi } from 'vitest'; - -import TodayPointsBadge from './TodayPointsBadge'; - -const mockUseTodayPoints = vi.fn<() => { data: { points_today: number | null } | undefined }>( - () => ({ data: { points_today: 12 } }) -); - -vi.mock('../../hooks/usePlayer', () => ({ - useTodayPoints: () => mockUseTodayPoints(), -})); - -describe('TodayPointsBadge', () => { - it("shows the player's points earned today", () => { - mockUseTodayPoints.mockReturnValue({ data: { points_today: 12 } }); - - render(); - - expect(screen.getByText('You contributed 12 today')).toBeInTheDocument(); - }); - - it('renders nothing when the player has no active character link', () => { - mockUseTodayPoints.mockReturnValue({ data: { points_today: null } }); - - const { container } = render(); - - expect(container).toBeEmptyDOMElement(); - }); - - it('renders nothing (not a zero) while the query has no data yet', () => { - mockUseTodayPoints.mockReturnValue({ data: undefined }); - - const { container } = render(); - - expect(container).toBeEmptyDOMElement(); - }); - - it('shows zero points as a real value, not treating it as "no link"', () => { - mockUseTodayPoints.mockReturnValue({ data: { points_today: 0 } }); - - render(); - - expect(screen.getByText('You contributed 0 today')).toBeInTheDocument(); - }); -}); diff --git a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.tsx b/frontend/src/components/TodayPointsBadge/TodayPointsBadge.tsx deleted file mode 100644 index c2cb5cc4..00000000 --- a/frontend/src/components/TodayPointsBadge/TodayPointsBadge.tsx +++ /dev/null @@ -1,23 +0,0 @@ -import React from "react"; -import { useTodayPoints } from "../../hooks/usePlayer"; -import styles from "./TodayPointsBadge.module.scss"; - -// Personal "points earned today" badge for the map view (issue #673) - -// deliberately not attached to any village marker, and deliberately silent -// about the player/character link: framed as "you contributed today" rather -// than naming a character, since players don't yet know about the link. -// Renders nothing (not a zero) when the player has no active -// PlayerCharacterLink - see MeViewSet.today_points in api/views.py. -export default function TodayPointsBadge(): React.ReactElement | null { - const { data } = useTodayPoints(); - - if (data?.points_today == null) { - return null; - } - - return ( -
    - You contributed {data.points_today} today -
    - ); -} diff --git a/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.module.scss b/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.module.scss index 2793e6af..db2234e2 100644 --- a/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.module.scss +++ b/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.module.scss @@ -4,7 +4,7 @@ @use '../../styles/semantic/spacing' as sp; @use 'sass:map'; -$row-control-height: 44px; +$row-control-height: sp.$form-control-height; .wrapper { position: relative; @@ -180,25 +180,13 @@ $row-control-height: 44px; } @include m.respond-to(md, down) { - .timerPill, - .ctaButton { - height: 40px; - } - .inputText { - height: 40px; font-size: 1rem; } } @include m.respond-to(sm, down) { - .timerPill, - .ctaButton { - height: 38px; - } - .inputText { - height: 38px; font-size: 0.9rem; } } diff --git a/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.tsx b/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.tsx index 8065af35..7b0b5b88 100644 --- a/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.tsx +++ b/frontend/src/components/UnifiedTimerHome/UnifiedTimerHome.tsx @@ -1,6 +1,7 @@ import React, { useEffect, useRef, useState } from "react"; import classNames from "classnames"; import { AnimatePresence, motion } from "framer-motion"; +import { formatDuration } from "../../utils/formatUtils"; import Button from "../Button/Button"; import AlertDialog from "../AlertDialog/AlertDialog"; @@ -40,8 +41,7 @@ export default function UnifiedTimerHome() { inputValue, taskId, activityCatalogId, - minutes, - seconds, + elapsed, formattedLimit, showAutoStopWarning, flowState, @@ -191,7 +191,7 @@ export default function UnifiedTimerHome() { {isActive && ( - {minutes}:{seconds.toString().padStart(2, "0")} + {formatDuration(elapsed)} )} diff --git a/frontend/src/config.ts b/frontend/src/config.ts index 5cf9b01f..586d481d 100644 --- a/frontend/src/config.ts +++ b/frontend/src/config.ts @@ -1,16 +1,14 @@ // src/config.ts -let API_BASE_URL: string; -const envApiBaseUrl = import.meta.env.VITE_API_BASE_URL as string | undefined; +function resolveApiBaseUrl(): string { + const envApiBaseUrl = import.meta.env.VITE_API_BASE_URL as string | undefined; -if (envApiBaseUrl) { - API_BASE_URL = envApiBaseUrl; -} else if (window.location.hostname === 'localhost') { - API_BASE_URL = 'http://localhost:8000'; -} else { - API_BASE_URL = window.location.origin; -} + const base = + envApiBaseUrl ?? + (window.location.hostname === 'localhost' + ? 'http://localhost:8000' + : window.location.origin); -API_BASE_URL = API_BASE_URL.replace(/\/api\/v1\/?$/i, ''); -API_BASE_URL = API_BASE_URL.replace(/\/$/, ''); + return base.replace(/\/api\/v1\/?$/i, '').replace(/\/$/, ''); +} -export { API_BASE_URL }; +export const API_BASE_URL = resolveApiBaseUrl(); diff --git a/frontend/src/context/WebSocketContext.tsx b/frontend/src/context/WebSocketContext.tsx index 2c4d6c5a..aabc3b7d 100644 --- a/frontend/src/context/WebSocketContext.tsx +++ b/frontend/src/context/WebSocketContext.tsx @@ -10,7 +10,7 @@ import { handleGlobalWebSocketEvent } from '../websockets/handleGlobalWebSocketE import { useMaintenanceStatus } from '../hooks/useMaintenanceStatus'; import { useMaintenanceContext } from './MaintenanceContext'; import { WebSocketContext } from './webSocketContext'; -import type { IncomingWebSocketMessage, OutgoingWebSocketMessage } from '../types'; +import type { ActivityTimerApiData, IncomingWebSocketMessage, OutgoingWebSocketMessage } from '../types'; // --------------------------------------------------------------------------- // Types @@ -30,7 +30,7 @@ const HEARTBEAT_INTERVAL_MS = 60_000; // --------------------------------------------------------------------------- export const WebSocketProvider = ({ children }: ProviderProps): ReactElement => { - const { player } = useGame(); + const { player, activityTimer, freeTimerLimitSeconds } = useGame(); const { setOnlinePlayerCount } = useOnlineCount(); const { isAuthenticated, loading: authLoading } = useAuth(); const { showToast } = useToast(); @@ -40,14 +40,26 @@ export const WebSocketProvider = ({ children }: ProviderProps): ReactElement => const eventHandlersRef = useRef void>>(new Set()); const wsEnabled = Boolean(!authLoading && isAuthenticated && player?.id); + const { loadFromServer } = activityTimer; + const onActivityTimerUpdate = useCallback((activityTimerData: ActivityTimerApiData) => { + // Reconciles this session's timer to the authoritative state pushed + // whenever another of the player's open sessions (tabs/devices) starts, + // labels, or submits the activity timer. loadFromServer just overwrites + // local state, so applying it to the session that originated the change + // (an echo of its own update) is harmless. + loadFromServer(activityTimerData, { + limitSeconds: player?.is_premium ? null : freeTimerLimitSeconds, + }); + }, [loadFromServer, player?.is_premium, freeTimerLimitSeconds]); + const onMessage = useCallback((data: IncomingWebSocketMessage) => { if (data.type === 'online_count') { setOnlinePlayerCount(data.count); } //console.log("[WS Provider] showToast:", showToast); - handleGlobalWebSocketEvent(data, { showToast, maintenanceRefetch, setMaintenance }); + handleGlobalWebSocketEvent(data, { showToast, maintenanceRefetch, setMaintenance, onActivityTimerUpdate }); eventHandlersRef.current.forEach((handler) => handler(data)); - }, [showToast, maintenanceRefetch, setMaintenance, setOnlinePlayerCount]); + }, [showToast, maintenanceRefetch, setMaintenance, setOnlinePlayerCount, onActivityTimerUpdate]); const onError = useCallback(() => { console.error('WebSocket connection error'); diff --git a/frontend/src/featureFlags.ts b/frontend/src/featureFlags.ts index a20f67b1..b19fd0af 100644 --- a/frontend/src/featureFlags.ts +++ b/frontend/src/featureFlags.ts @@ -16,7 +16,7 @@ const featureFlags: Record = { unified_homepage: [], results_mode: [], map: ['testers'], - todayPointsBadge: ['all'], + dailyGoalsBadge: ['testers'], }; export default featureFlags; diff --git a/frontend/src/hooks/useActivities.ts b/frontend/src/hooks/useActivities.ts index 17f6718c..0e05fc4d 100644 --- a/frontend/src/hooks/useActivities.ts +++ b/frontend/src/hooks/useActivities.ts @@ -1,7 +1,7 @@ // src/hooks/useActivities.ts import { useMutation, useQueryClient, useQuery } from "@tanstack/react-query"; -import { updateActivity, deleteActivity, fetchActivities, createActivity } from "../api/activities"; +import { updateActivity, deleteActivity, fetchActivities, createActivity, logOfflineActivity } from "../api/activities"; import type { PlayerActivity } from "../types"; @@ -26,6 +26,20 @@ export function useCreateActivity() { } +export function useLogOfflineActivity() { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: logOfflineActivity, + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: ["activities"] }); + // A task may have been auto-created (or its total time updated) by the log. + queryClient.invalidateQueries({ queryKey: ["tasks"] }); + }, + }); +} + + export function useUpdateActivity() { const queryClient = useQueryClient(); diff --git a/frontend/src/hooks/usePlayer.ts b/frontend/src/hooks/usePlayer.ts index 6773eedf..11e12ea2 100644 --- a/frontend/src/hooks/usePlayer.ts +++ b/frontend/src/hooks/usePlayer.ts @@ -1,27 +1,27 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useNavigate } from "react-router"; -import { updatePlayer, downloadUserData, deleteAccount, fetchTodayPoints } from "../api/player"; +import { updatePlayer, downloadUserData, deleteAccount, fetchDailyGoals } from "../api/player"; import { useAuth } from "../context/AuthContext"; // Query key shared with useActivityInput's refreshAfterActivityChange, which -// invalidates it after every completed task/session so the map view's -// "today" badge (issue #673) updates immediately instead of waiting for the -// next poll. -export const TODAY_POINTS_QUERY_KEY = ["me", "today-points"]; +// invalidates it after every completed task/session so the map view's daily +// goals badge (issue #673, redesigned in #751) updates immediately instead +// of waiting for the next poll. +export const DAILY_GOALS_QUERY_KEY = ["me", "daily-goals"]; // Polled lightly (rather than one-shot like other /me/ data) so the badge // still rolls over to the new day's value on its own if the map view is left // open across midnight, without needing a dedicated push mechanism for that. -const TODAY_POINTS_POLL_INTERVAL_MS = 60_000; +const DAILY_GOALS_POLL_INTERVAL_MS = 60_000; -export function useTodayPoints() { +export function useDailyGoals() { const { isAuthenticated } = useAuth(); return useQuery({ - queryKey: TODAY_POINTS_QUERY_KEY, - queryFn: fetchTodayPoints, + queryKey: DAILY_GOALS_QUERY_KEY, + queryFn: fetchDailyGoals, enabled: isAuthenticated, - refetchInterval: TODAY_POINTS_POLL_INTERVAL_MS, + refetchInterval: DAILY_GOALS_POLL_INTERVAL_MS, }); } diff --git a/frontend/src/hooks/useSimpleCrudPanel.ts b/frontend/src/hooks/useSimpleCrudPanel.ts index 9baa8297..e5437581 100644 --- a/frontend/src/hooks/useSimpleCrudPanel.ts +++ b/frontend/src/hooks/useSimpleCrudPanel.ts @@ -8,6 +8,11 @@ interface Entity { name: string; } +interface SaveCallbacks { + onSuccess?: () => void; + onError?: () => void; +} + interface UseSimpleCrudPanelOptions { useList: () => Pick, "data" | "isLoading">; useCreate: () => Pick>, "mutate">; @@ -41,8 +46,8 @@ export function useSimpleCrudPanel({ ); const handleEdit = useCallback( - (item: T, name: string) => { - update.mutate({ id: item.id, data: { name } as Partial }); + (item: T, name: string, callbacks?: SaveCallbacks) => { + update.mutate({ id: item.id, data: { name } as Partial }, callbacks); }, [update], ); diff --git a/frontend/src/pages/MapPage/MapPage.tsx b/frontend/src/pages/MapPage/MapPage.tsx index 04a35d76..45e65503 100644 --- a/frontend/src/pages/MapPage/MapPage.tsx +++ b/frontend/src/pages/MapPage/MapPage.tsx @@ -5,7 +5,7 @@ import Button from "../../components/Button/Button"; import PopulationCentreMap, { type PopulationCentreMapHandle, } from "../../components/Map/Map"; -import TodayPointsBadge from "../../components/TodayPointsBadge/TodayPointsBadge"; +import DailyGoalsBadge from "../../components/DailyGoalsBadge/DailyGoalsBadge"; import { fetchPopulationCentreMap } from "../../api/map"; import FeatureToggle from "../../components/FeatureToggle"; import { @@ -136,8 +136,8 @@ export default function MapPage(): React.ReactElement { onViewportChange={handleViewportChange} worldBounds={worldBounds?.bbox} > - - + + {nextVillage && (