From 2c6d9e431c42a0d9b69fe4c6b25e8eb651941492 Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Fri, 31 Jul 2026 01:56:23 +0200 Subject: [PATCH 01/21] feat(shopping-lists): Replace isPublic with a revocable share token Changes: - Add a ListAccess enum (NONE, VIEW, SHOP, EDIT, OWNER) with capability helpers - Replace shopping_list.is_public with a nullable link_access column and a unique share_token, minted on enable and nulled on disable - Add ShoppingListAccessService as the single authorization rule - Add SharedShoppingListService and /api/shared/{token} for read, rename and item update or delete, permit-all at the filter chain - Extract ShoppingListMapper, retiring the item mapping duplicated across ShoppingListService and ShoppingListItemService - Drop the "or the list is public" write fallbacks, making the authenticated endpoints strictly owner-only - Bind and verify the previously ignored {listId} on item update and delete isPublic was unreachable speculative code that granted write access to any logged-in stranger holding the list UUID, and it could never be revoked because the link was the primary key. A separate token means turning sharing off and on again actually invalidates the old link. Anonymous callers resolve to VIEW at most, so every write stays attributable to an account. Notes: - Requires ALTER TABLE shopping_list DROP COLUMN is_public; before deploy. ddl-auto=update never drops columns and is_public is NOT NULL with no default, so inserts fail until it is gone. - Verified with javac against the local m2 repository: 81 sources, no errors. Maven build not run. --- .../java/disscount/config/SecurityConfig.java | 3 + .../dao/ShoppingListRepository.java | 3 + .../shoppingList/domain/ListAccess.java | 35 +++++ .../shoppingList/domain/ShoppingList.java | 18 ++- .../shoppingList/dto/ShoppingListDto.java | 11 +- .../shoppingList/dto/ShoppingListRequest.java | 6 +- .../rest/SharedShoppingListController.java | 80 ++++++++++ .../service/SharedShoppingListService.java | 148 ++++++++++++++++++ .../service/ShoppingListAccessService.java | 34 ++++ .../service/ShoppingListMapper.java | 61 ++++++++ .../service/ShoppingListService.java | 83 +++++----- .../rest/ShoppingListItemController.java | 6 +- .../service/ShoppingListItemService.java | 78 ++++----- 13 files changed, 462 insertions(+), 104 deletions(-) create mode 100644 backend/src/main/java/disscount/shoppingList/domain/ListAccess.java create mode 100644 backend/src/main/java/disscount/shoppingList/rest/SharedShoppingListController.java create mode 100644 backend/src/main/java/disscount/shoppingList/service/SharedShoppingListService.java create mode 100644 backend/src/main/java/disscount/shoppingList/service/ShoppingListAccessService.java create mode 100644 backend/src/main/java/disscount/shoppingList/service/ShoppingListMapper.java diff --git a/backend/src/main/java/disscount/config/SecurityConfig.java b/backend/src/main/java/disscount/config/SecurityConfig.java index 73858259..68196949 100644 --- a/backend/src/main/java/disscount/config/SecurityConfig.java +++ b/backend/src/main/java/disscount/config/SecurityConfig.java @@ -46,6 +46,9 @@ public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) .authorizeHttpRequests(authz -> authz .requestMatchers(HttpMethod.POST, "/api/contact").permitAll() + // Shared lists authorize on the share token plus ShoppingListAccessService, + // not on the filter chain, because the caller may legitimately be anonymous. + .requestMatchers("/api/shared/**").permitAll() .requestMatchers( "/actuator/health", "/v3/api-docs/**", diff --git a/backend/src/main/java/disscount/shoppingList/dao/ShoppingListRepository.java b/backend/src/main/java/disscount/shoppingList/dao/ShoppingListRepository.java index ecf37224..d1207111 100644 --- a/backend/src/main/java/disscount/shoppingList/dao/ShoppingListRepository.java +++ b/backend/src/main/java/disscount/shoppingList/dao/ShoppingListRepository.java @@ -23,4 +23,7 @@ public interface ShoppingListRepository extends JpaRepository findActiveByIdAndOwner(UUID id, User owner); + + @Query("SELECT sl FROM ShoppingList sl WHERE sl.shareToken = :shareToken AND sl.deletedAt IS NULL") + Optional findActiveByShareToken(UUID shareToken); } diff --git a/backend/src/main/java/disscount/shoppingList/domain/ListAccess.java b/backend/src/main/java/disscount/shoppingList/domain/ListAccess.java new file mode 100644 index 00000000..91095b9a --- /dev/null +++ b/backend/src/main/java/disscount/shoppingList/domain/ListAccess.java @@ -0,0 +1,35 @@ +package disscount.shoppingList.domain; + +/** + * What a caller may do with a shopping list. + * + *

Only NONE, VIEW, SHOP and EDIT are ever stored in {@code shopping_list.link_access}. + * OWNER is produced by {@link disscount.shoppingList.service.ShoppingListAccessService} + * and never persisted. + */ +public enum ListAccess { + + NONE, + VIEW, + SHOP, + EDIT, + OWNER; + + public boolean canView() { + return this != NONE; + } + + /** The in-the-shop level: tick an item off, switch its store, snapshot its price. */ + public boolean canCheck() { + return this == SHOP || this == EDIT || this == OWNER; + } + + /** Amount, removal and the list title. Adding items arrives with membership. */ + public boolean canEditItems() { + return this == EDIT || this == OWNER; + } + + public boolean canManageShare() { + return this == OWNER; + } +} diff --git a/backend/src/main/java/disscount/shoppingList/domain/ShoppingList.java b/backend/src/main/java/disscount/shoppingList/domain/ShoppingList.java index 5f1533af..b4d81a8b 100644 --- a/backend/src/main/java/disscount/shoppingList/domain/ShoppingList.java +++ b/backend/src/main/java/disscount/shoppingList/domain/ShoppingList.java @@ -33,9 +33,16 @@ public class ShoppingList { @Column(nullable = false) private String title; - @Column(name = "is_public", nullable = false) - @Builder.Default - private Boolean isPublic = false; + // Nullable because ddl-auto=update cannot add a NOT NULL column to a populated table. + // Read it through resolvedLinkAccess(), never directly. + @Enumerated(EnumType.STRING) + @Column(name = "link_access", length = 16) + private ListAccess linkAccess; + + // Deliberately not the list id: a token can be rotated, so turning sharing off and on + // again actually revokes instead of handing the same URL back to everyone who kept it. + @Column(name = "share_token", unique = true) + private UUID shareToken; @Column(name = "updated_at", nullable = false) private LocalDateTime updatedAt; @@ -50,6 +57,11 @@ public class ShoppingList { @Builder.Default private List items = new ArrayList<>(); + /** Null link access means the list is not shared at all. */ + public ListAccess resolvedLinkAccess() { + return linkAccess != null ? linkAccess : ListAccess.NONE; + } + @PrePersist protected void onCreate() { LocalDateTime now = LocalDateTime.now(); diff --git a/backend/src/main/java/disscount/shoppingList/dto/ShoppingListDto.java b/backend/src/main/java/disscount/shoppingList/dto/ShoppingListDto.java index 8619c329..61da5940 100644 --- a/backend/src/main/java/disscount/shoppingList/dto/ShoppingListDto.java +++ b/backend/src/main/java/disscount/shoppingList/dto/ShoppingListDto.java @@ -7,6 +7,7 @@ import java.util.List; import java.util.UUID; +import disscount.shoppingList.domain.ListAccess; import disscount.shoppingListItem.dto.ShoppingListItemDto; @Data @@ -16,7 +17,15 @@ public class ShoppingListDto { private UUID id; private UUID ownerId; private String title; - private Boolean isPublic; + + // Both owner-only: a link visitor handed the token could reshare the list at a level + // its owner never granted. + private ListAccess linkAccess; + private UUID shareToken; + + /** The caller's resolved access, echoed back so the frontend never re-derives the rule. */ + private ListAccess myAccess; + private LocalDateTime updatedAt; private LocalDateTime createdAt; private List items; diff --git a/backend/src/main/java/disscount/shoppingList/dto/ShoppingListRequest.java b/backend/src/main/java/disscount/shoppingList/dto/ShoppingListRequest.java index 97207b04..24653f19 100644 --- a/backend/src/main/java/disscount/shoppingList/dto/ShoppingListRequest.java +++ b/backend/src/main/java/disscount/shoppingList/dto/ShoppingListRequest.java @@ -3,11 +3,15 @@ import jakarta.validation.constraints.NotBlank; import lombok.Data; +import disscount.shoppingList.domain.ListAccess; + @Data public class ShoppingListRequest { @NotBlank(message = "Title is required") private String title; - private Boolean isPublic = false; + // Owner-only, and ignored on create: sharing is turned on from an existing list, + // because there is no id to bind a token to until the list has been saved. + private ListAccess linkAccess; } diff --git a/backend/src/main/java/disscount/shoppingList/rest/SharedShoppingListController.java b/backend/src/main/java/disscount/shoppingList/rest/SharedShoppingListController.java new file mode 100644 index 00000000..b5d683db --- /dev/null +++ b/backend/src/main/java/disscount/shoppingList/rest/SharedShoppingListController.java @@ -0,0 +1,80 @@ +package disscount.shoppingList.rest; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.tags.Tag; +import jakarta.validation.Valid; +import lombok.RequiredArgsConstructor; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.*; + +import disscount.shoppingList.dto.ShoppingListDto; +import disscount.shoppingList.dto.ShoppingListRequest; +import disscount.shoppingList.service.SharedShoppingListService; +import disscount.shoppingListItem.dto.ShoppingListItemDto; +import disscount.shoppingListItem.dto.ShoppingListItemRequest; +import disscount.util.SecurityUtils; + +import java.util.UUID; + +/** + * Public entry point for shared lists. Permit-all at the filter chain; the real check is the + * token plus {@link disscount.shoppingList.service.ShoppingListAccessService}. + * + *

Callers may be anonymous, so every method reads the user through + * {@code getCurrentUserIdOptional()} and never {@code getCurrentUserId()}, which throws. + */ +@RestController +@RequestMapping("/api/shared") +@RequiredArgsConstructor +@Tag(name = "Shared Shopping Lists", description = "Shopping lists reachable by share token") +public class SharedShoppingListController { + + private final SharedShoppingListService sharedShoppingListService; + + @Operation(summary = "Get a shared shopping list by its share token") + @GetMapping("/{token}") + public ResponseEntity getSharedShoppingList(@PathVariable String token) { + UUID userId = currentUserId(); + return sharedShoppingListService.getByToken(token, userId) + .map(ResponseEntity::ok) + .orElse(ResponseEntity.notFound().build()); + } + + @Operation(summary = "Rename a shared shopping list") + @PutMapping("/{token}") + public ResponseEntity updateSharedShoppingList( + @PathVariable String token, + @Valid @RequestBody ShoppingListRequest request) { + UUID userId = currentUserId(); + return sharedShoppingListService.updateTitle(token, userId, request) + .map(ResponseEntity::ok) + .orElse(ResponseEntity.notFound().build()); + } + + @Operation(summary = "Update an item on a shared shopping list") + @PutMapping("/{token}/items/{itemId}") + public ResponseEntity updateSharedItem( + @PathVariable String token, + @PathVariable UUID itemId, + @Valid @RequestBody ShoppingListItemRequest request) { + UUID userId = currentUserId(); + return sharedShoppingListService.updateItem(token, itemId, userId, request) + .map(ResponseEntity::ok) + .orElse(ResponseEntity.notFound().build()); + } + + @Operation(summary = "Delete an item from a shared shopping list") + @DeleteMapping("/{token}/items/{itemId}") + public ResponseEntity deleteSharedItem( + @PathVariable String token, + @PathVariable UUID itemId) { + UUID userId = currentUserId(); + return sharedShoppingListService.deleteItem(token, itemId, userId) + ? ResponseEntity.noContent().build() + : ResponseEntity.notFound().build(); + } + + private UUID currentUserId() { + return SecurityUtils.getCurrentUserIdOptional().orElse(null); + } +} diff --git a/backend/src/main/java/disscount/shoppingList/service/SharedShoppingListService.java b/backend/src/main/java/disscount/shoppingList/service/SharedShoppingListService.java new file mode 100644 index 00000000..304243ef --- /dev/null +++ b/backend/src/main/java/disscount/shoppingList/service/SharedShoppingListService.java @@ -0,0 +1,148 @@ +package disscount.shoppingList.service; + +import lombok.RequiredArgsConstructor; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +import disscount.exceptions.ForbiddenException; +import disscount.exceptions.UnauthorizedException; +import disscount.shoppingList.dao.ShoppingListRepository; +import disscount.shoppingList.domain.ListAccess; +import disscount.shoppingList.domain.ShoppingList; +import disscount.shoppingList.dto.ShoppingListDto; +import disscount.shoppingList.dto.ShoppingListRequest; +import disscount.shoppingListItem.dao.ShoppingListItemRepository; +import disscount.shoppingListItem.domain.ShoppingListItem; +import disscount.shoppingListItem.dto.ShoppingListItemDto; +import disscount.shoppingListItem.dto.ShoppingListItemRequest; +import disscount.user.dao.UserRepository; +import disscount.user.domain.User; + +import java.time.LocalDateTime; +import java.util.Optional; +import java.util.UUID; +import java.util.function.Predicate; + +/** + * Everything reachable through a share token. The token is the capability, so it travels on + * every call here and knowing a list's id is never enough on its own. + * + *

An unresolvable token yields an empty Optional, which the controller turns into a 404 + * rather than a 403, so the endpoint never confirms that a token once existed. + */ +@Service +@RequiredArgsConstructor +@Transactional +public class SharedShoppingListService { + + private final ShoppingListRepository shoppingListRepository; + private final ShoppingListItemRepository shoppingListItemRepository; + private final UserRepository userRepository; + private final ShoppingListAccessService accessService; + private final ShoppingListMapper shoppingListMapper; + + @Transactional(readOnly = true) + public Optional getByToken(String token, UUID userId) { + return findShared(token) + .map(list -> shoppingListMapper.toDto(list, accessService.resolve(list, userId))); + } + + public Optional updateTitle(String token, UUID userId, ShoppingListRequest request) { + return findShared(token).map(list -> { + ListAccess access = requireAccess(list, userId, ListAccess::canEditItems); + + list.setTitle(request.getTitle()); + return shoppingListMapper.toDto(shoppingListRepository.save(list), access); + }); + } + + public Optional updateItem( + String token, UUID itemId, UUID userId, ShoppingListItemRequest request) { + return findShared(token).flatMap(list -> { + ListAccess access = requireAccess(list, userId, ListAccess::canCheck); + User actor = requireUser(userId); + + return shoppingListItemRepository.findActiveByIdAndShoppingList(itemId, list).map(item -> { + applyItemUpdate(item, request, access); + item.setUpdatedAt(LocalDateTime.now()); + item.setUpdatedByUser(actor); + + ShoppingListItem saved = shoppingListItemRepository.save(item); + touchList(list); + return shoppingListMapper.toItemDto(saved); + }); + }); + } + + public boolean deleteItem(String token, UUID itemId, UUID userId) { + return findShared(token).map(list -> { + requireAccess(list, userId, ListAccess::canEditItems); + + return shoppingListItemRepository.findActiveByIdAndShoppingList(itemId, list).map(item -> { + item.setDeletedAt(LocalDateTime.now()); + shoppingListItemRepository.save(item); + touchList(list); + return true; + }).orElse(false); + }).orElse(false); + } + + /** + * A list is reachable by token only while it is actually shared. The token is nulled + * whenever link access goes back to NONE, so this is belt and braces. + */ + private Optional findShared(String token) { + UUID parsed; + try { + parsed = UUID.fromString(token); + } catch (IllegalArgumentException ex) { + // A malformed token is indistinguishable from an unknown one, by design. + return Optional.empty(); + } + + return shoppingListRepository.findActiveByShareToken(parsed) + .filter(list -> list.resolvedLinkAccess() != ListAccess.NONE); + } + + private ListAccess requireAccess(ShoppingList list, UUID userId, Predicate allowed) { + ListAccess access = accessService.resolve(list, userId); + if (!allowed.test(access)) { + throw new ForbiddenException("Insufficient access to this shopping list"); + } + return access; + } + + private User requireUser(UUID userId) { + return userRepository.findById(userId) + .orElseThrow(() -> new UnauthorizedException("User not found")); + } + + /** + * SHOP is the in-the-shop level: what got ticked, which store it is coming from, and the + * prices captured at that moment. Everything structural is left as the server has it, so a + * SHOP-level caller cannot rename or resize an item by sending a fuller payload. + */ + private void applyItemUpdate(ShoppingListItem item, ShoppingListItemRequest request, ListAccess access) { + item.setIsChecked(request.getIsChecked() != null ? request.getIsChecked() : false); + item.setChainCode(request.getChainCode()); + item.setAvgPrice(request.getAvgPrice()); + item.setStorePrice(request.getStorePrice()); + + if (!access.canEditItems()) { + return; + } + + item.setEan(request.getEan()); + item.setBrand(request.getBrand()); + item.setName(request.getName()); + item.setQuantity(request.getQuantity()); + item.setUnit(request.getUnit()); + item.setAmount(request.getAmount() != null ? request.getAmount() : 1); + } + + /** Item activity reorders the owner's list index, which sorts by updatedAt. */ + private void touchList(ShoppingList list) { + list.setUpdatedAt(LocalDateTime.now()); + shoppingListRepository.save(list); + } +} diff --git a/backend/src/main/java/disscount/shoppingList/service/ShoppingListAccessService.java b/backend/src/main/java/disscount/shoppingList/service/ShoppingListAccessService.java new file mode 100644 index 00000000..48616699 --- /dev/null +++ b/backend/src/main/java/disscount/shoppingList/service/ShoppingListAccessService.java @@ -0,0 +1,34 @@ +package disscount.shoppingList.service; + +import org.springframework.stereotype.Service; + +import disscount.shoppingList.domain.ListAccess; +import disscount.shoppingList.domain.ShoppingList; + +import java.util.UUID; + +/** + * The single authorization rule for shopping lists. Membership adds a + * max(member permission, link access) branch here and nowhere else. + */ +@Service +public class ShoppingListAccessService { + + /** + * @param userId the caller, or null when the request carries no token at all + */ + public ListAccess resolve(ShoppingList list, UUID userId) { + if (userId != null && userId.equals(list.getOwner().getId())) { + return ListAccess.OWNER; + } + + ListAccess link = list.resolvedLinkAccess(); + if (link == ListAccess.NONE) { + return ListAccess.NONE; + } + + // Anonymous callers are capped at VIEW however generous the link is, which is what + // keeps every write attributable to an account. + return userId == null ? ListAccess.VIEW : link; + } +} diff --git a/backend/src/main/java/disscount/shoppingList/service/ShoppingListMapper.java b/backend/src/main/java/disscount/shoppingList/service/ShoppingListMapper.java new file mode 100644 index 00000000..53ac1f69 --- /dev/null +++ b/backend/src/main/java/disscount/shoppingList/service/ShoppingListMapper.java @@ -0,0 +1,61 @@ +package disscount.shoppingList.service; + +import org.springframework.stereotype.Component; + +import disscount.shoppingList.domain.ListAccess; +import disscount.shoppingList.domain.ShoppingList; +import disscount.shoppingList.dto.ShoppingListDto; +import disscount.shoppingListItem.domain.ShoppingListItem; +import disscount.shoppingListItem.dto.ShoppingListItemDto; + +import java.util.List; +import java.util.stream.Collectors; + +/** + * One place items and lists become DTOs, so the owner view and the shared view cannot + * drift apart in what they expose. + */ +@Component +public class ShoppingListMapper { + + public ShoppingListDto toDto(ShoppingList list, ListAccess access) { + List items = list.getItems().stream() + .filter(item -> item.getDeletedAt() == null) + .map(this::toItemDto) + .collect(Collectors.toList()); + + boolean isOwner = access == ListAccess.OWNER; + + return ShoppingListDto.builder() + .id(list.getId()) + .ownerId(list.getOwner().getId()) + .title(list.getTitle()) + .linkAccess(isOwner ? list.resolvedLinkAccess() : null) + .shareToken(isOwner ? list.getShareToken() : null) + .myAccess(access) + .updatedAt(list.getUpdatedAt()) + .createdAt(list.getCreatedAt()) + .items(items) + .build(); + } + + public ShoppingListItemDto toItemDto(ShoppingListItem item) { + return ShoppingListItemDto.builder() + .id(item.getId()) + .shoppingListId(item.getShoppingList().getId()) + .ean(item.getEan()) + .brand(item.getBrand()) + .name(item.getName()) + .quantity(item.getQuantity()) + .unit(item.getUnit()) + .amount(item.getAmount()) + .isChecked(item.getIsChecked()) + .chainCode(item.getChainCode()) + .avgPrice(item.getAvgPrice()) + .storePrice(item.getStorePrice()) + .createdAt(item.getCreatedAt()) + .updatedAt(item.getUpdatedAt()) + .updatedByUserId(item.getUpdatedByUser() != null ? item.getUpdatedByUser().getId() : null) + .build(); + } +} diff --git a/backend/src/main/java/disscount/shoppingList/service/ShoppingListService.java b/backend/src/main/java/disscount/shoppingList/service/ShoppingListService.java index e9b4f49f..3326d196 100644 --- a/backend/src/main/java/disscount/shoppingList/service/ShoppingListService.java +++ b/backend/src/main/java/disscount/shoppingList/service/ShoppingListService.java @@ -7,10 +7,10 @@ import disscount.exceptions.BadRequestException; import disscount.exceptions.UnauthorizedException; import disscount.shoppingList.dao.ShoppingListRepository; +import disscount.shoppingList.domain.ListAccess; import disscount.shoppingList.domain.ShoppingList; import disscount.shoppingList.dto.ShoppingListDto; import disscount.shoppingList.dto.ShoppingListRequest; -import disscount.shoppingListItem.dto.ShoppingListItemDto; import disscount.user.dao.UserRepository; import disscount.user.domain.User; @@ -20,6 +20,10 @@ import java.util.UUID; import java.util.stream.Collectors; +/** + * The owner's view of their own lists. Everything here is owner-only; access granted by a + * share link runs through {@link SharedShoppingListService} instead. + */ @Service @RequiredArgsConstructor @Transactional @@ -27,19 +31,21 @@ public class ShoppingListService { private final ShoppingListRepository shoppingListRepository; private final UserRepository userRepository; + private final ShoppingListMapper shoppingListMapper; public ShoppingListDto createShoppingList(UUID ownerId, ShoppingListRequest request) { User owner = userRepository.findById(ownerId) .orElseThrow(() -> new UnauthorizedException("User not found")); + // New lists are always private, which also makes "copy list" private by construction. + // Sharing needs a persisted id to bind a token to, so it is turned on afterwards. ShoppingList shoppingList = ShoppingList.builder() .owner(owner) .title(request.getTitle()) - .isPublic(request.getIsPublic() != null ? request.getIsPublic() : false) .build(); shoppingList = shoppingListRepository.save(shoppingList); - return convertToDto(shoppingList); + return shoppingListMapper.toDto(shoppingList, ListAccess.OWNER); } public List getUserShoppingLists(UUID ownerId) { @@ -48,7 +54,7 @@ public List getUserShoppingLists(UUID ownerId) { return shoppingListRepository.findActiveByOwner(owner) .stream() - .map(this::convertToDto) + .map(list -> shoppingListMapper.toDto(list, ListAccess.OWNER)) .collect(Collectors.toList()); } @@ -56,16 +62,8 @@ public Optional getShoppingListById(UUID listId, UUID ownerId) User owner = userRepository.findById(ownerId) .orElseThrow(() -> new UnauthorizedException("User not found")); - // First try to get as owner - Optional shoppingListOpt = shoppingListRepository.findActiveByIdAndOwner(listId, owner); - - // If not found and not owner, try to get public list - if (shoppingListOpt.isEmpty()) { - shoppingListOpt = shoppingListRepository.findActiveById(listId) - .filter(list -> list.getIsPublic()); - } - - return shoppingListOpt.map(this::convertToDto); + return shoppingListRepository.findActiveByIdAndOwner(listId, owner) + .map(list -> shoppingListMapper.toDto(list, ListAccess.OWNER)); } public ShoppingListDto updateShoppingList(UUID listId, UUID ownerId, ShoppingListRequest request) { @@ -75,12 +73,11 @@ public ShoppingListDto updateShoppingList(UUID listId, UUID ownerId, ShoppingLis ShoppingList shoppingList = shoppingListRepository.findActiveByIdAndOwner(listId, owner) .orElseThrow(() -> new BadRequestException("Shopping list not found")); - // Update fields shoppingList.setTitle(request.getTitle()); - shoppingList.setIsPublic(request.getIsPublic() != null ? request.getIsPublic() : false); + applyLinkAccess(shoppingList, request.getLinkAccess()); shoppingList = shoppingListRepository.save(shoppingList); - return convertToDto(shoppingList); + return shoppingListMapper.toDto(shoppingList, ListAccess.OWNER); } public void deleteShoppingList(UUID listId, UUID ownerId) { @@ -94,36 +91,28 @@ public void deleteShoppingList(UUID listId, UUID ownerId) { shoppingListRepository.save(shoppingList); } - private ShoppingListDto convertToDto(ShoppingList shoppingList) { - List itemDtos = shoppingList.getItems().stream() - .filter(item -> item.getDeletedAt() == null) - .map(item -> ShoppingListItemDto.builder() - .id(item.getId()) - .shoppingListId(item.getShoppingList().getId()) - .ean(item.getEan()) - .brand(item.getBrand()) - .name(item.getName()) - .quantity(item.getQuantity()) - .unit(item.getUnit()) - .amount(item.getAmount()) - .isChecked(item.getIsChecked()) - .chainCode(item.getChainCode()) - .avgPrice(item.getAvgPrice()) - .storePrice(item.getStorePrice()) - .createdAt(item.getCreatedAt()) - .updatedAt(item.getUpdatedAt()) - .updatedByUserId(item.getUpdatedByUser() != null ? item.getUpdatedByUser().getId() : null) - .build()) - .collect(Collectors.toList()); + /** + * Re-enabling a link mints a fresh token, so turning sharing off and on again is a real + * revoke. Merely changing the level leaves the token alone, since the people already + * holding the link are meant to keep working at the new level. + */ + private void applyLinkAccess(ShoppingList list, ListAccess requested) { + if (requested == null || requested == list.resolvedLinkAccess()) { + return; + } + if (requested == ListAccess.OWNER) { + throw new BadRequestException("OWNER is not a link access level"); + } - return ShoppingListDto.builder() - .id(shoppingList.getId()) - .ownerId(shoppingList.getOwner().getId()) - .title(shoppingList.getTitle()) - .isPublic(shoppingList.getIsPublic()) - .updatedAt(shoppingList.getUpdatedAt()) - .createdAt(shoppingList.getCreatedAt()) - .items(itemDtos) - .build(); + if (requested == ListAccess.NONE) { + list.setLinkAccess(null); + list.setShareToken(null); + return; + } + + list.setLinkAccess(requested); + if (list.getShareToken() == null) { + list.setShareToken(UUID.randomUUID()); + } } } diff --git a/backend/src/main/java/disscount/shoppingListItem/rest/ShoppingListItemController.java b/backend/src/main/java/disscount/shoppingListItem/rest/ShoppingListItemController.java index a68ef2e4..dd940788 100644 --- a/backend/src/main/java/disscount/shoppingListItem/rest/ShoppingListItemController.java +++ b/backend/src/main/java/disscount/shoppingListItem/rest/ShoppingListItemController.java @@ -35,19 +35,21 @@ public ResponseEntity addItemToShoppingList( @Operation(summary = "Update shopping list item") @PutMapping("/{itemId}") public ResponseEntity updateShoppingListItem( + @PathVariable UUID listId, @PathVariable UUID itemId, @Valid @RequestBody ShoppingListItemRequest request) { UUID ownerId = SecurityUtils.getCurrentUserId(); - ShoppingListItemDto updated = shoppingListItemService.updateShoppingListItem(itemId, ownerId, request); + ShoppingListItemDto updated = shoppingListItemService.updateShoppingListItem(listId, itemId, ownerId, request); return ResponseEntity.ok(updated); } @Operation(summary = "Delete shopping list item") @DeleteMapping("/{itemId}") public ResponseEntity deleteShoppingListItem( + @PathVariable UUID listId, @PathVariable UUID itemId) { UUID ownerId = SecurityUtils.getCurrentUserId(); - shoppingListItemService.deleteShoppingListItem(itemId, ownerId); + shoppingListItemService.deleteShoppingListItem(listId, itemId, ownerId); return ResponseEntity.noContent().build(); } } diff --git a/backend/src/main/java/disscount/shoppingListItem/service/ShoppingListItemService.java b/backend/src/main/java/disscount/shoppingListItem/service/ShoppingListItemService.java index fe704526..0fbba99f 100644 --- a/backend/src/main/java/disscount/shoppingListItem/service/ShoppingListItemService.java +++ b/backend/src/main/java/disscount/shoppingListItem/service/ShoppingListItemService.java @@ -8,6 +8,7 @@ import disscount.exceptions.UnauthorizedException; import disscount.shoppingList.dao.ShoppingListRepository; import disscount.shoppingList.domain.ShoppingList; +import disscount.shoppingList.service.ShoppingListMapper; import disscount.shoppingListItem.dao.ShoppingListItemRepository; import disscount.shoppingListItem.domain.ShoppingListItem; import disscount.shoppingListItem.dto.ShoppingListItemDto; @@ -21,6 +22,11 @@ import java.util.UUID; import java.util.stream.Collectors; +/** + * The owner's own items. Writes granted by a share link go through + * {@link disscount.shoppingList.service.SharedShoppingListService} instead, so that the token + * has to travel with the request. + */ @Service @RequiredArgsConstructor @Transactional @@ -29,16 +35,14 @@ public class ShoppingListItemService { private final ShoppingListItemRepository shoppingListItemRepository; private final ShoppingListRepository shoppingListRepository; private final UserRepository userRepository; + private final ShoppingListMapper shoppingListMapper; public ShoppingListItemDto addItemToShoppingList(UUID shoppingListId, UUID ownerId, ShoppingListItemRequest request) { User owner = userRepository.findById(ownerId) .orElseThrow(() -> new UnauthorizedException("User not found")); - // First try to get as owner, then as public list ShoppingList shoppingList = shoppingListRepository.findActiveByIdAndOwner(shoppingListId, owner) - .orElseGet(() -> shoppingListRepository.findActiveById(shoppingListId) - .filter(list -> list.getIsPublic()) - .orElseThrow(() -> new BadRequestException("Shopping list not found or access denied"))); + .orElseThrow(() -> new BadRequestException("Shopping list not found or access denied")); // Check if item with same name already exists in the shopping list Optional existingItem = shoppingListItemRepository @@ -52,7 +56,7 @@ public ShoppingListItemDto addItemToShoppingList(UUID shoppingListId, UUID owner // Cap the merged total at the same limit the request DTO enforces (@Max) int newAmount = Math.min(item.getAmount() + requestedAmount, 999); item.setAmount(newAmount); - + // Update other fields with new values if provided if (request.getEan() != null) item.setEan(request.getEan()); if (request.getBrand() != null) item.setBrand(request.getBrand()); @@ -61,7 +65,7 @@ public ShoppingListItemDto addItemToShoppingList(UUID shoppingListId, UUID owner if (request.getChainCode() != null) item.setChainCode(request.getChainCode()); if (request.getAvgPrice() != null) item.setAvgPrice(request.getAvgPrice()); if (request.getStorePrice() != null) item.setStorePrice(request.getStorePrice()); - + // Update tracking fields item.setUpdatedAt(LocalDateTime.now()); item.setUpdatedByUser(owner); @@ -89,23 +93,14 @@ public ShoppingListItemDto addItemToShoppingList(UUID shoppingListId, UUID owner shoppingList.setUpdatedAt(LocalDateTime.now()); shoppingListRepository.save(shoppingList); - return convertToDto(item); + return shoppingListMapper.toItemDto(item); } - public ShoppingListItemDto updateShoppingListItem(UUID itemId, UUID ownerId, ShoppingListItemRequest request) { - userRepository.findById(ownerId) + public ShoppingListItemDto updateShoppingListItem(UUID listId, UUID itemId, UUID ownerId, ShoppingListItemRequest request) { + User owner = userRepository.findById(ownerId) .orElseThrow(() -> new UnauthorizedException("User not found")); - ShoppingListItem item = shoppingListItemRepository.findActiveById(itemId) - .filter(i -> { - // Allow if user is owner OR if list is public - return i.getShoppingList().getOwner().getId().equals(ownerId) || - i.getShoppingList().getIsPublic(); - }) - .orElseThrow(() -> new BadRequestException("Shopping list item not found or access denied")); - - User currentUser = userRepository.findById(ownerId) - .orElseThrow(() -> new UnauthorizedException("User not found")); + ShoppingListItem item = findOwnedItem(listId, itemId, owner); // Update fields item.setEan(request.getEan()); @@ -118,10 +113,10 @@ public ShoppingListItemDto updateShoppingListItem(UUID itemId, UUID ownerId, Sho item.setChainCode(request.getChainCode()); item.setAvgPrice(request.getAvgPrice()); item.setStorePrice(request.getStorePrice()); - + // Update tracking fields item.setUpdatedAt(LocalDateTime.now()); - item.setUpdatedByUser(currentUser); + item.setUpdatedByUser(owner); item = shoppingListItemRepository.save(item); @@ -129,20 +124,14 @@ public ShoppingListItemDto updateShoppingListItem(UUID itemId, UUID ownerId, Sho item.getShoppingList().setUpdatedAt(LocalDateTime.now()); shoppingListRepository.save(item.getShoppingList()); - return convertToDto(item); + return shoppingListMapper.toItemDto(item); } - public void deleteShoppingListItem(UUID itemId, UUID ownerId) { - userRepository.findById(ownerId) + public void deleteShoppingListItem(UUID listId, UUID itemId, UUID ownerId) { + User owner = userRepository.findById(ownerId) .orElseThrow(() -> new UnauthorizedException("User not found")); - ShoppingListItem item = shoppingListItemRepository.findActiveById(itemId) - .filter(i -> { - // Allow if user is owner OR if list is public - return i.getShoppingList().getOwner().getId().equals(ownerId) || - i.getShoppingList().getIsPublic(); - }) - .orElseThrow(() -> new BadRequestException("Shopping list item not found or access denied")); + ShoppingListItem item = findOwnedItem(listId, itemId, owner); item.setDeletedAt(LocalDateTime.now()); shoppingListItemRepository.save(item); @@ -158,27 +147,16 @@ public List getUserShoppingListItems(UUID ownerId) { return shoppingListItemRepository.findAllActiveItemsByUser(owner) .stream() - .map(this::convertToDto) + .map(shoppingListMapper::toItemDto) .collect(Collectors.toList()); } - private ShoppingListItemDto convertToDto(ShoppingListItem item) { - return ShoppingListItemDto.builder() - .id(item.getId()) - .shoppingListId(item.getShoppingList().getId()) - .ean(item.getEan()) - .brand(item.getBrand()) - .name(item.getName()) - .quantity(item.getQuantity()) - .unit(item.getUnit()) - .amount(item.getAmount()) - .isChecked(item.getIsChecked()) - .chainCode(item.getChainCode()) - .avgPrice(item.getAvgPrice()) - .storePrice(item.getStorePrice()) - .createdAt(item.getCreatedAt()) - .updatedAt(item.getUpdatedAt()) - .updatedByUserId(item.getUpdatedByUser() != null ? item.getUpdatedByUser().getId() : null) - .build(); + /** The item has to belong both to the list in the path and to the caller. */ + private ShoppingListItem findOwnedItem(UUID listId, UUID itemId, User owner) { + ShoppingList shoppingList = shoppingListRepository.findActiveByIdAndOwner(listId, owner) + .orElseThrow(() -> new BadRequestException("Shopping list not found or access denied")); + + return shoppingListItemRepository.findActiveByIdAndShoppingList(itemId, shoppingList) + .orElseThrow(() -> new BadRequestException("Shopping list item not found or access denied")); } } From 55175c6f941ed26c4c991c6125bd1b2a0cdb4eb9 Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Fri, 31 Jul 2026 02:27:10 +0200 Subject: [PATCH 02/21] feat(shopping-lists): Wire the frontend to link access and share tokens Changes: - Replace isPublic in the zod schemas with linkAccess, shareToken and myAccess, and clear the three creation call sites that hardcoded it - Add shared-list fetchers and hooks against /api/shared/{token} - Key the shared read as ["sharedShoppingList", token], deliberately outside the offline persistence allowlist - Add resolveShoppingListAccess, mirroring the backend rule from myAccess - Add shareListUrl, building /s/{token} rather than exposing the list id - Register sharedItemUpdate and sharedItemDelete as offline mutations, with an onError on their replay defaults - Give the visibility indicator three shared states instead of two - Bump the offline CACHE_BUSTER to "2" The persisted ShoppingListDto shape changed, so a restored pre-change list would have no myAccess and every capability check would read that as no access. Hence the buster bump. Copying a list deliberately does not carry sharing over, and now says so in its toast. A copy is a new object, and inheriting a capability token would mint a live secret nobody chose to hand out. This matches AnyList, Todoist, Notion, Trello, Keep and Drive; Asana is the only mainstream product that inherits, and that is a team ACL rather than a secret link. Notes: - constants/loading-labels.ts and lib/api/shopping-lists/keys.ts are taken verbatim from feat/button-loading-labels and feat/loading-skeletons so those branches rebase onto this one as no-op hunks. keys.ts adds one byToken entry. - feat/loading-skeletons should use CACHE_BUSTER "3", not "2". - Checks: tsc --noEmit clean, eslint 0 errors, prettier applied. The 27 eslint warnings are pre-existing and in untouched files. --- .../[id]/components/shopping-list-header.tsx | 4 +- .../[id]/hooks/use-shopping-list-actions.ts | 12 +-- .../[id]/hooks/use-shopping-list-mutations.ts | 16 +++- .../components/forms/shopping-list-modal.tsx | 6 +- .../components/shopping-list-item.tsx | 8 +- .../shopping-list-visibility-indicator.tsx | 44 +++++++-- .../shopping-lists/utils/share-list-url.ts | 10 +++ .../utils/shopping-list-access.ts | 29 ++++++ .../create-discounted-list-button.tsx | 1 - .../products/hooks/use-add-to-list-submit.ts | 5 +- frontend/src/constants/loading-labels.ts | 24 +++++ frontend/src/lib/api/schemas/shopping-list.ts | 22 ++++- frontend/src/lib/api/shopping-lists/hooks.ts | 90 ++++++++++++++++--- frontend/src/lib/api/shopping-lists/keys.ts | 20 +++++ .../src/lib/api/shopping-lists/queries.ts | 41 +++++++++ .../src/lib/offline/offline-mutation-keys.ts | 4 + frontend/src/lib/offline/offline-mutations.ts | 42 +++++++++ frontend/src/lib/offline/persister.ts | 5 +- 18 files changed, 341 insertions(+), 42 deletions(-) create mode 100644 frontend/src/app/(user)/shopping-lists/utils/share-list-url.ts create mode 100644 frontend/src/app/(user)/shopping-lists/utils/shopping-list-access.ts create mode 100644 frontend/src/constants/loading-labels.ts create mode 100644 frontend/src/lib/api/shopping-lists/keys.ts diff --git a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-header.tsx b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-header.tsx index 4a6462ee..e80d44d3 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-header.tsx +++ b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-header.tsx @@ -44,7 +44,9 @@ export default function ShoppingListHeader({

- + ({ resolver: zodResolver(shoppingListRequestSchema), mode: "onChange", - defaultValues: { title: "", isPublic: false }, + // No linkAccess here on purpose: sharing lives in its own modal, and the backend + // treats an absent linkAccess on PUT as "leave it alone", so renaming a shared list + // from here cannot silently unshare it. + defaultValues: { title: "" }, }); // Destructured, never read inline: formState is a Proxy that subscribes to a @@ -73,7 +76,6 @@ export default function ShoppingListModal({ const base = { title: shoppingList.title, - isPublic: shoppingList.isPublic ?? false, }; form.reset(base); diff --git a/frontend/src/app/(user)/shopping-lists/components/shopping-list-item.tsx b/frontend/src/app/(user)/shopping-lists/components/shopping-list-item.tsx index 5242c97d..b34e7b90 100644 --- a/frontend/src/app/(user)/shopping-lists/components/shopping-list-item.tsx +++ b/frontend/src/app/(user)/shopping-lists/components/shopping-list-item.tsx @@ -41,7 +41,9 @@ export default function ShoppingListListItem({
- + @@ -76,7 +78,9 @@ export default function ShoppingListListItem({
- + @@ -27,11 +57,7 @@ export default function ShoppingListVisibilityIndicator({ role="img" aria-label={label} > - {isPublic ? ( -
{/* Right side: Amount controls, price, and remove button */} @@ -96,7 +105,11 @@ export default function ShoppingListItem({
- +
{/* Store Chain Select */} @@ -109,7 +122,7 @@ export default function ShoppingListItem({ chainCode, }) } - disabled={item.isChecked} + disabled={item.isChecked || !canCheck} defaultValue={cheapestStore} storePrices={storePrices} averagePrice={averagePrice} @@ -120,11 +133,13 @@ export default function ShoppingListItem({
{/* Remove button - hidden on mobile, shown on larger screens */} - + {canEditItems && ( + + )} diff --git a/frontend/src/app/(user)/shopping-lists/[id]/components/items/shopping-list-items.tsx b/frontend/src/app/(user)/shopping-lists/[id]/components/items/shopping-list-items.tsx index bc7908c3..ed66177b 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/components/items/shopping-list-items.tsx +++ b/frontend/src/app/(user)/shopping-lists/[id]/components/items/shopping-list-items.tsx @@ -15,6 +15,7 @@ import { cn } from "@/lib/utils"; import ShoppingListItem from "@/app/(user)/shopping-lists/[id]/components/items/shopping-list-item"; import type { ShoppingListDto as ShoppingList } from "@/lib/api/types"; import { useShoppingListItemMutations } from "@/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-item-mutations"; +import { resolveShoppingListAccess } from "@/app/(user)/shopping-lists/utils/shopping-list-access"; import { getShoppingListItemsOpen, setShoppingListItemsOpen, @@ -25,6 +26,8 @@ interface IShoppingListItemsProps { cheapestStores: Record; averagePrices: Record; storePrices: Record>; + /** Set when this list was opened through a share link, so writes carry the token. */ + shareToken?: string; } export default function ShoppingListItems({ @@ -32,9 +35,19 @@ export default function ShoppingListItems({ cheapestStores, averagePrices, storePrices, + shareToken, }: IShoppingListItemsProps) { + const { canCheck, canEditItems, isOwner } = resolveShoppingListAccess( + shoppingList.myAccess, + ); + const { handleUpdateItem, handleDeleteItem, deletingItemId } = - useShoppingListItemMutations(shoppingList.id, averagePrices, storePrices); + useShoppingListItemMutations( + shoppingList.id, + averagePrices, + storePrices, + shareToken, + ); const [isItemsOpen, setIsItemsOpen] = useState(() => getShoppingListItemsOpen(shoppingList.id), @@ -85,16 +98,20 @@ export default function ShoppingListItems({ {shoppingList.items.length === 0 ? (

- Ovaj popis još ne sadrži proizvode. Pretraži proizvode i dodaj ih - na ovaj popis. + {isOwner + ? "Ovaj popis još ne sadrži proizvode. Pretraži proizvode i dodaj ih na ovaj popis." + : "Ovaj popis još ne sadrži proizvode."}

- + {/* Only the owner can add items, so nobody else gets an invitation to try. */} + {isOwner && ( + + )}
) : ( @@ -114,6 +131,8 @@ export default function ShoppingListItems({ isFirst={index === 0} isLast={index === sortedItems.length - 1} showSeparator={index < sortedItems.length - 1} + canCheck={canCheck} + canEditItems={canEditItems} /> ))} diff --git a/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-item-mutations.ts b/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-item-mutations.ts index d0fafe87..b2ba268b 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-item-mutations.ts +++ b/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-item-mutations.ts @@ -2,18 +2,33 @@ import { useState } from "react"; import { useQueryClient } from "@tanstack/react-query"; import { toast } from "sonner"; import { shoppingListService } from "@/lib/api"; +import { SHOPPING_LIST_QUERY_KEYS } from "@/lib/api/shopping-lists/keys"; import type { ShoppingListDto as ShoppingList } from "@/lib/api/types"; +/** + * @param shareToken present when the list was reached through a share link, in which case + * writes go to /api/shared/{token}: the token is the capability, so knowing the list id + * is never enough on its own. + */ export function useShoppingListItemMutations( listId: string, averagePrices: Record, storePrices: Record>, + shareToken?: string, ) { const queryClient = useQueryClient(); const [deletingItemId, setDeletingItemId] = useState(null); const updateItemMutation = shoppingListService.useUpdateShoppingListItem(); const deleteItemMutation = shoppingListService.useDeleteShoppingListItem(); + const updateSharedItemMutation = + shoppingListService.useUpdateSharedShoppingListItem(); + const deleteSharedItemMutation = + shoppingListService.useDeleteSharedShoppingListItem(); + + const queryKey = shareToken + ? SHOPPING_LIST_QUERY_KEYS.byToken(shareToken) + : SHOPPING_LIST_QUERY_KEYS.byId(listId); const handleUpdateItem = async ( itemId: string, @@ -23,10 +38,7 @@ export function useShoppingListItemMutations( chainCode: string | null; }, ) => { - const shoppingList = queryClient.getQueryData([ - "shoppingLists", - listId, - ]); + const shoppingList = queryClient.getQueryData(queryKey); const item = shoppingList?.items?.find((i) => i.id === itemId); if (!item) return; @@ -35,35 +47,29 @@ export function useShoppingListItemMutations( if (updatedItem.amount < 1) return; // Optimistic update - await queryClient.cancelQueries({ queryKey: ["shoppingLists", listId] }); - const previousData = queryClient.getQueryData([ - "shoppingLists", - listId, - ]); - - queryClient.setQueryData( - ["shoppingLists", listId], - (old) => { - if (!old) return old; - return { - ...old, - items: old.items?.map((i) => { - if (i.id === itemId) { - const updated = { ...i, ...updatedItem }; - // If checking the item, include the current average price - if (updatedItem.isChecked) { - const currentAvgPrice = averagePrices[i.id]; - if (currentAvgPrice !== undefined) { - updated.avgPrice = currentAvgPrice; - } + await queryClient.cancelQueries({ queryKey }); + const previousData = queryClient.getQueryData(queryKey); + + queryClient.setQueryData(queryKey, (old) => { + if (!old) return old; + return { + ...old, + items: old.items?.map((i) => { + if (i.id === itemId) { + const updated = { ...i, ...updatedItem }; + // If checking the item, include the current average price + if (updatedItem.isChecked) { + const currentAvgPrice = averagePrices[i.id]; + if (currentAvgPrice !== undefined) { + updated.avgPrice = currentAvgPrice; } - return updated; } - return i; - }), - }; - }, - ); + return updated; + } + return i; + }), + }; + }); // Prepare update data const updateData = { @@ -87,6 +93,22 @@ export function useShoppingListItemMutations( } } + function rollback() { + if (previousData) { + queryClient.setQueryData(queryKey, previousData); + } + } + + if (shareToken) { + updateSharedItemMutation.mutate( + { token: shareToken, itemId, data: updateData }, + // No toast here: the shared mutation's offline defaults already carry one, and it + // is the only handler that survives a replay after a reload. + { onError: rollback }, + ); + return; + } + updateItemMutation.mutate( { listId, @@ -95,9 +117,7 @@ export function useShoppingListItemMutations( }, { onError: (error: Error) => { - if (previousData) { - queryClient.setQueryData(["shoppingLists", listId], previousData); - } + rollback(); toast.error( error.message || "Greška pri ažuriranju stavke. Pokušaj ponovno.", ); @@ -110,31 +130,41 @@ export function useShoppingListItemMutations( setDeletingItemId(itemId); // Optimistic update - await queryClient.cancelQueries({ queryKey: ["shoppingLists", listId] }); - const previousData = queryClient.getQueryData([ - "shoppingLists", - listId, - ]); - - queryClient.setQueryData( - ["shoppingLists", listId], - (old) => { - if (!old) return old; - return { - ...old, - items: old.items?.filter((i) => i.id !== itemId), - }; - }, - ); + await queryClient.cancelQueries({ queryKey }); + const previousData = queryClient.getQueryData(queryKey); + + queryClient.setQueryData(queryKey, (old) => { + if (!old) return old; + return { + ...old, + items: old.items?.filter((i) => i.id !== itemId), + }; + }); + + function rollback() { + if (previousData) { + queryClient.setQueryData(queryKey, previousData); + } + } + + if (shareToken) { + deleteSharedItemMutation.mutate( + { token: shareToken, itemId }, + { + onError: rollback, + onSuccess: () => toast.success("Stavka je uspješno obrisana!"), + onSettled: () => setDeletingItemId(null), + }, + ); + return; + } // Delete the item deleteItemMutation.mutate( { listId, itemId }, { onError: (error: Error) => { - if (previousData) { - queryClient.setQueryData(["shoppingLists", listId], previousData); - } + rollback(); toast.error( error.message || "Greška pri brisanju stavke. Pokušaj ponovno.", ); From e7503569d1bb09facd6a5fe917c7e4cb59080b58 Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Fri, 31 Jul 2026 02:34:17 +0200 Subject: [PATCH 04/21] feat(shopping-lists): Add the share settings modal Changes: - Add ?modal=shopping-list/share, a settings panel that saves on change rather than behind a submit button, with a sharing switch, the three access levels and the link with a copy button - Keep "Podijeli kao tekst" in the same modal, so the existing plain-text share is relocated rather than lost - Point the owner's share action at the modal; everyone else still gets the direct share, with the link they already hold or plain text - Hide edit, delete and the visibility badge from non-owners - Extract components/custom/common/copy-button.tsx and retire the two identical copyEmail helpers in admin-contact-row and contact-channels - Split the shopping-list and digital-card cases in parseModalParam and entity-modal-outlet, which the share action needs anyway Sharing has to save immediately because the server mints the token: there is no link to show until a save returns, so a submit button would leave the primary content of the dialog empty until pressed. Notes: - The action components now derive their labels the way feat/button-loading-labels does, and the modal-registry and outlet splits match feat/digital-cards-rework, so both rebase onto this branch as no-op hunks. - Checks: tsc --noEmit clean, eslint clean on changed files, prettier applied. --- .../shopping-list-desktop-actions.tsx | 19 ++- .../[id]/components/shopping-list-header.tsx | 17 ++- .../shopping-list-mobile-actions.tsx | 13 +- .../[id]/hooks/use-shopping-list-actions.ts | 16 +- .../components/forms/share-list-modal.tsx | 142 ++++++++++++++++++ .../hooks/use-share-list-modal.ts | 70 +++++++++ .../components/admin-contact-row.tsx | 28 +--- .../components/custom/common/copy-button.tsx | 64 ++++++++ .../custom/contact/contact-channels.tsx | 28 +--- .../modal-router/entity-modal-outlet.tsx | 25 ++- frontend/src/lib/modal/modal-registry.ts | 5 + 11 files changed, 364 insertions(+), 63 deletions(-) create mode 100644 frontend/src/app/(user)/shopping-lists/components/forms/share-list-modal.tsx create mode 100644 frontend/src/app/(user)/shopping-lists/hooks/use-share-list-modal.ts create mode 100644 frontend/src/components/custom/common/copy-button.tsx diff --git a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-desktop-actions.tsx b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-desktop-actions.tsx index a39aa3ec..21743e70 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-desktop-actions.tsx +++ b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-desktop-actions.tsx @@ -7,6 +7,7 @@ import { TooltipTrigger, } from "@/components/ui/tooltip"; import { cn } from "@/lib/utils"; +import { LOADING_LABELS } from "@/constants/loading-labels"; import type { IShoppingListActionGroupProps } from "@/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-actions"; interface IShoppingListDesktopActionsProps extends IShoppingListActionGroupProps { @@ -29,6 +30,12 @@ export default function ShoppingListDesktopActions({ visibleOnMobile = false, className, }: IShoppingListDesktopActionsProps) { + // Icon-only, so the spinner is the whole visual and the accessible name carries the + // pending copy. The tooltip has to say the same thing or the two contradict each other. + const shareLabel = isSharing ? LOADING_LABELS.sharing : "Podijeli popis"; + const copyLabel = isCopying ? LOADING_LABELS.copying : "Kopiraj popis"; + const deleteLabel = isDeleting ? LOADING_LABELS.deleting : "Obriši popis"; + return (
+
+ )} + + ); +} diff --git a/frontend/src/app/(user)/shopping-lists/hooks/use-share-list-modal.ts b/frontend/src/app/(user)/shopping-lists/hooks/use-share-list-modal.ts new file mode 100644 index 00000000..19d0b29a --- /dev/null +++ b/frontend/src/app/(user)/shopping-lists/hooks/use-share-list-modal.ts @@ -0,0 +1,70 @@ +"use client"; + +import { useState } from "react"; +import { toast } from "sonner"; + +import { shoppingListService } from "@/lib/api"; +import type { LinkAccess } from "@/lib/api/types"; +import { shareOrCopy } from "@/utils/browser/share"; +import { formatShoppingListForSharing } from "@/app/(user)/shopping-lists/utils/shopping-list-utils"; +import { shareListUrl } from "@/app/(user)/shopping-lists/utils/share-list-url"; + +/** + * Share settings save on change rather than behind a submit button: the server mints the + * token, so there is no link to show until a save has come back. + */ +export function useShareListModal(id: string) { + const [isSharingText, setIsSharingText] = useState(false); + + const listQuery = shoppingListService.useGetShoppingListById(id); + const updateMutation = shoppingListService.useUpdateShoppingList(); + + const shoppingList = listQuery.data ?? null; + const linkAccess: LinkAccess = shoppingList?.linkAccess ?? "NONE"; + const shareUrl = shoppingList?.shareToken + ? shareListUrl(shoppingList.shareToken) + : null; + + function setLinkAccess(next: LinkAccess) { + if (!shoppingList || next === linkAccess) return; + + // PUT carries the whole request, so the current title has to ride along or the + // server would reject it as blank. + updateMutation.mutate( + { id, data: { title: shoppingList.title, linkAccess: next } }, + { + onError: () => + toast.error("Promjena dijeljenja nije spremljena. Pokušaj ponovno."), + }, + ); + } + + async function handleTextShare() { + if (!shoppingList) return; + + setIsSharingText(true); + try { + const outcome = await shareOrCopy({ + title: shoppingList.title, + text: formatShoppingListForSharing(shoppingList), + }); + + if (outcome === "copied") toast.success("Tekst popisa je kopiran"); + if (outcome === "failed") toast.error("Dijeljenje nije uspjelo"); + } finally { + setIsSharingText(false); + } + } + + return { + shoppingList, + isLoading: listQuery.isLoading, + isError: listQuery.isError, + linkAccess, + setLinkAccess, + isSaving: updateMutation.isPending, + shareUrl, + handleTextShare, + isSharingText, + }; +} diff --git a/frontend/src/app/dashboard/components/admin-contact-row.tsx b/frontend/src/app/dashboard/components/admin-contact-row.tsx index 4c57d820..10071e74 100644 --- a/frontend/src/app/dashboard/components/admin-contact-row.tsx +++ b/frontend/src/app/dashboard/components/admin-contact-row.tsx @@ -1,10 +1,10 @@ "use client"; -import { Copy, Mail, MailOpen, RotateCcw, Trash2 } from "lucide-react"; -import { toast } from "sonner"; +import { Mail, MailOpen, RotateCcw, Trash2 } from "lucide-react"; import { Badge } from "@/components/ui/badge"; import { Button } from "@/components/ui/button"; +import CopyButton from "@/components/custom/common/copy-button"; import { TableCell, TableRow } from "@/components/ui/table"; import { formatDate } from "@/utils/strings"; import { ContactMessageDto } from "@/lib/api/types"; @@ -28,15 +28,6 @@ export default function AdminContactRow({ const isDeleted = !!message.deletedAt; const email = message.email ?? ""; - async function copyEmail() { - try { - await navigator.clipboard.writeText(email); - toast.success("E-mail adresa je kopirana!"); - } catch { - toast.error("Greška pri kopiranju e-maila"); - } - } - return ( @@ -84,15 +75,12 @@ export default function AdminContactRow({ {email && ( - + )} {isDeleted ? ( diff --git a/frontend/src/components/custom/common/copy-button.tsx b/frontend/src/components/custom/common/copy-button.tsx new file mode 100644 index 00000000..3987fe3e --- /dev/null +++ b/frontend/src/components/custom/common/copy-button.tsx @@ -0,0 +1,64 @@ +"use client"; + +import { useEffect, useRef, useState } from "react"; +import { Check, Copy } from "lucide-react"; +import { toast } from "sonner"; + +import { Button } from "@/components/ui/button"; +import { cn } from "@/lib/utils"; + +interface ICopyButtonProps { + value: string; + /** Accessible name and, when shown, the tooltip. Name what is copied, not "Kopiraj". */ + label: string; + successMessage: string; + errorMessage?: string; + className?: string; +} + +const CONFIRMED_MS = 2000; + +/** Ghost icon button that copies `value` to the clipboard and confirms it. */ +export default function CopyButton({ + value, + label, + successMessage, + errorMessage = "Greška pri kopiranju", + className, +}: ICopyButtonProps) { + const [copied, setCopied] = useState(false); + const timeoutRef = useRef | null>(null); + + useEffect(() => { + return () => { + if (timeoutRef.current) clearTimeout(timeoutRef.current); + }; + }, []); + + async function handleCopy() { + try { + await navigator.clipboard.writeText(value); + // The toast announces it; the icon swap is for anyone who is looking rather + // than listening. + toast.success(successMessage); + setCopied(true); + if (timeoutRef.current) clearTimeout(timeoutRef.current); + timeoutRef.current = setTimeout(() => setCopied(false), CONFIRMED_MS); + } catch { + toast.error(errorMessage); + } + } + + return ( + + ); +} diff --git a/frontend/src/components/custom/contact/contact-channels.tsx b/frontend/src/components/custom/contact/contact-channels.tsx index 998dfcd9..91d5dc8f 100644 --- a/frontend/src/components/custom/contact/contact-channels.tsx +++ b/frontend/src/components/custom/contact/contact-channels.tsx @@ -1,21 +1,12 @@ "use client"; import Link from "next/link"; -import { Bug, Copy, ExternalLink, Lightbulb } from "lucide-react"; -import { toast } from "sonner"; +import { Bug, ExternalLink, Lightbulb } from "lucide-react"; import { Button } from "@/components/ui/button"; +import CopyButton from "@/components/custom/common/copy-button"; import { CONTACT_EMAIL, LINKEDIN_URL } from "@/constants/contact"; -async function copyEmail() { - try { - await navigator.clipboard.writeText(CONTACT_EMAIL); - toast.success("E-mail adresa je kopirana!"); - } catch { - toast.error("Greška pri kopiranju e-maila"); - } -} - /** Intro line plus links to the dedicated idea and bug flows. */ export default function ContactChannels() { return ( @@ -24,15 +15,12 @@ export default function ContactChannels() { Pošalji nam poruku kroz obrazac ispod, direktno na{" "} {CONTACT_EMAIL} - + {" "} ili putem{" "} + + + ); + } + + return ( +
+ {!isUserLoading && !isAuthenticated && ( + +

+ Prijavi se za uređivanje ovog popisa. +

+ + + )} + +
+ + + {listUpdatedAt > 0 && ( + + )} +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+ +
+
+ ); +} diff --git a/frontend/src/app/s/[token]/get-shared-list-preview.ts b/frontend/src/app/s/[token]/get-shared-list-preview.ts new file mode 100644 index 00000000..d346c62c --- /dev/null +++ b/frontend/src/app/s/[token]/get-shared-list-preview.ts @@ -0,0 +1,43 @@ +import "server-only"; + +import type { ShoppingListDto } from "@/lib/api/types"; + +/** + * Server-side read used only for the link preview. It goes straight to the backend origin + * rather than through the Next rewrite, which only rewrites browser requests. + * + * Never cached: a revoked link has to stop previewing immediately, and the rendered HTML + * is also excluded from the service worker's page cache in sw.ts for the same reason. + */ +export async function getSharedListPreview( + token: string, +): Promise { + const origin = process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:8080"; + + try { + const response = await fetch( + `${origin}/api/shared/${encodeURIComponent(token)}`, + { cache: "no-store" }, + ); + + if (!response.ok) return null; + + return (await response.json()) as ShoppingListDto; + } catch { + // A preview is a nicety; a backend blip must not take the page down with it. + return null; + } +} + +/** Croatian counts: 1 stavka, 2 to 4 stavke, 5 or more stavki, ignoring the teens. */ +export function formatItemCount(count: number): string { + const lastTwo = count % 100; + const last = count % 10; + + if (lastTwo < 11 || lastTwo > 14) { + if (last === 1) return `${count} stavka`; + if (last >= 2 && last <= 4) return `${count} stavke`; + } + + return `${count} stavki`; +} diff --git a/frontend/src/app/s/[token]/page.tsx b/frontend/src/app/s/[token]/page.tsx new file mode 100644 index 00000000..c6c429d6 --- /dev/null +++ b/frontend/src/app/s/[token]/page.tsx @@ -0,0 +1,47 @@ +import { Metadata } from "next"; + +import SharedShoppingListClient from "@/app/s/[token]/components/shared-shopping-list-client"; +import { + formatItemCount, + getSharedListPreview, +} from "@/app/s/[token]/get-shared-list-preview"; + +// A shared list is unlisted, not public: it should preview nicely when pasted into a chat +// and never turn up in a search result. next.config.ts sends X-Robots-Tag for the same +// reason, since a robots.txt rule would stop a crawler ever seeing this. +const NOINDEX = { index: false, follow: false } as const; + +export async function generateMetadata( + props: PageProps<"/s/[token]">, +): Promise { + const { token } = await props.params; + const shoppingList = await getSharedListPreview(token); + + if (!shoppingList) { + return { + title: "Podijeljeni popis za kupnju", + robots: NOINDEX, + }; + } + + const description = `Popis za kupnju, ${formatItemCount(shoppingList.items.length)}.`; + + return { + title: shoppingList.title, + description, + robots: NOINDEX, + openGraph: { + title: shoppingList.title, + description, + type: "website", + }, + }; +} + +export default async function SharedShoppingListPage( + props: PageProps<"/s/[token]">, +) { + const { token } = await props.params; + + return ; +} diff --git a/frontend/src/app/sw.ts b/frontend/src/app/sw.ts index 53fa34f5..73d69e52 100644 --- a/frontend/src/app/sw.ts +++ b/frontend/src/app/sw.ts @@ -44,6 +44,14 @@ const runtimeCaching: RuntimeCaching[] = [ sameOrigin && url.pathname.startsWith("/api/"), handler: new NetworkOnly(), }, + // Shared lists server-render someone else's list title for the link preview, and + // defaultCache would keep that document for 24 days keyed by URL alone, with no notion + // of who asked. Must stay above defaultCache, which is matched in order. + { + matcher: ({ url, sameOrigin }) => + sameOrigin && url.pathname.startsWith("/s/"), + handler: new NetworkOnly(), + }, ...defaultCache, ]; From 9593f1a11488963f87a6196f71041d8dfdf2e202 Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Fri, 31 Jul 2026 02:39:11 +0200 Subject: [PATCH 06/21] fix(shopping-lists): Drop dead-end controls for logged-out link visitors Changes: - Hide the copy action from the shared page unless the visitor is signed in - Hide the "back to shopping lists" link for the same reason Copying calls POST /api/shopping-lists, so for an anonymous visitor the button was a guaranteed 401, and the list index it linked back to is itself behind a login gate. Both were reachable only through a share link, which is why they survived until the page existed. --- .../[id]/components/shopping-list-header.tsx | 40 +++++++++++-------- .../shared-shopping-list-client.tsx | 5 ++- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-header.tsx b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-header.tsx index ba53a1fe..44b6f365 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-header.tsx +++ b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-header.tsx @@ -13,10 +13,16 @@ import { interface IShoppingListHeaderProps { shoppingList: ShoppingList; + /** + * False for a logged-out link visitor: copying creates a list on their own account, + * and the list index they would go back to is itself behind a login. + */ + isSignedIn?: boolean; } export default function ShoppingListHeader({ shoppingList, + isSignedIn = true, }: IShoppingListHeaderProps) { const { isOwner } = resolveShoppingListAccess(shoppingList.myAccess); @@ -24,22 +30,24 @@ export default function ShoppingListHeader({
- - - - + {isSignedIn && ( + + + + - - Natrag na popise za kupnju - - + + Natrag na popise za kupnju + + + )}

{shoppingList.title} @@ -56,7 +64,7 @@ export default function ShoppingListHeader({ )} - + {listUpdatedAt > 0 && ( Date: Wed, 5 Aug 2026 15:26:44 +0200 Subject: [PATCH 07/21] chore(agents): Hand back clickable paths and system-themed review HTML Changes: - Require every artifact path to be absolute, resolved with realpath, and on its own line - Record the worktree trap: reviews/ is gitignored, so no git operation moves the file - Make the HTML variant follow prefers-color-scheme, with dark as the base palette - Force light in @media print so PDF export stays legible - Require every colour to be a CSS variable, with a snippet that greps for stray literals - Add a slug to the review filename so several runs stay distinguishable The last review was handed over as a light-only page and as a path into the worktree it was generated in, while the reader was in the main checkout. Both are avoidable by rule rather than by remembering. Dark is the base rather than the override so an absent or unknown preference lands on dark. Notes: This commit is unrelated to shopping list sharing and is meant to be cherry-picked onto dev. --- .../skills/multi-tool-code-review/SKILL.md | 2 + .../references/03-triage-doc-format.md | 48 +++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/.claude/skills/multi-tool-code-review/SKILL.md b/.claude/skills/multi-tool-code-review/SKILL.md index b6e529ad..08970392 100644 --- a/.claude/skills/multi-tool-code-review/SKILL.md +++ b/.claude/skills/multi-tool-code-review/SKILL.md @@ -88,6 +88,8 @@ Follow `04-fix-protocol.md`. If the harness supports plan mode, enter it first a ## Conventions (apply throughout) - Ask if you are unsure of anything rather than assuming. Follow the host repo's `AGENTS.md` / `CLAUDE.md` closely. +- **Always hand back full absolute paths, on their own line.** Every artifact you write (the triage doc in each format, and any raw runner output you point at) gets its real path via `realpath`, never a bare filename or a repo-relative fragment buried in a sentence. The user clicks these to open them, and a path that is not absolute is not clickable. Reviews often run from a git worktree while the user sits in the main checkout, so resolve the path instead of assuming a shared working directory, and say which checkout it is in. See "Delivering the doc" in `03-triage-doc-format.md`. +- **The HTML variant follows the system colour scheme, dark by default.** Base palette dark in `:root`, light via `@media (prefers-color-scheme: light)`, print forced light, every colour a CSS variable. Full rules in `03-triage-doc-format.md`. - No em dashes anywhere (chat, docs, commits, comments). - Do not hardcode any model; ask the user each run and recommend from a fresh online check. - Frontend gate: `pnpm exec prettier --write ` then `pnpm exec tsc --noEmit`. Run `pnpm exec next typegen` first and the typecheck is clean; without it, `tsc` reports `PageProps` / `RouteContext` errors that are missing generated route types rather than real defects. Before pushing, reproduce the full CI job from `.github/workflows/`, production build included. diff --git a/.claude/skills/multi-tool-code-review/references/03-triage-doc-format.md b/.claude/skills/multi-tool-code-review/references/03-triage-doc-format.md index 9602bbf0..9d92a324 100644 --- a/.claude/skills/multi-tool-code-review/references/03-triage-doc-format.md +++ b/.claude/skills/multi-tool-code-review/references/03-triage-doc-format.md @@ -77,3 +77,51 @@ End the doc by telling the user it is easier to name what NOT to fix than what t If prettier is available in the repo, run it on the Markdown so the tables align: `./node_modules/.bin/prettier --write "../reviews/REVIEW--BY-AREA.md"` from the frontend package. Prettier only checks table syntax, not content, so re-read the rows after any scripted (awk/sed) column edit to catch a swapped cell. For the HTML variant, build a single self-contained page with the `frontend-design` skill if one is available, so it is a readable, well-typeset document rather than a generic dump. Keep the same areas, columns, legend, and row numbering; the only goal of HTML is easier scanning of a long list. Never send the review to any external host; it stays a local file under `reviews/`. + +Self-contained means genuinely self-contained: inline the CSS, use system font stacks, and reference no CDN, webfont, or image. The page is opened over `file://`, often with no network, and anything external renders as a broken document. + +### The HTML must follow the system colour scheme + +Default to **dark**, and let a light system preference override it. Not the other way round: the user's environment is dark nearly all the time, so dark is the right base and the right fallback when the preference is unknown. + +Drive it entirely through CSS custom properties. Declare every colour once in `:root` as dark, then re-declare the same names inside `@media (prefers-color-scheme: light)`. Every rule below references `var(--x)` and never a literal, so the two palettes cannot drift: + +```css +:root { + --bg: #161614; --fg: #e8e6e1; --line: #2f2d29; /* ...dark is the base... */ +} +@media (prefers-color-scheme: light) { + :root { --bg:#fbfbfa; --fg:#1a1a19; --line:#e4e2dd; /* ...light overrides... */ } +} +@media print { + :root { --bg:#fff; --fg:#1a1a19; /* force light so PDF export is legible */ } +} +``` + +Severity and recommendation pills need a variable pair each (background and foreground), not one shared set: a light pill background with dark text is unreadable inverted, so the dark palette wants desaturated backgrounds with light text. Do not leave a single hardcoded hex in a rule body. + +Verify before handing it over, since a stray literal is invisible until the user's theme flips: + +```bash +python3 - <<'PY' +h = open("reviews/.html", encoding="utf-8").read() +css = h.split("")[0] +body = css.split("*{box-sizing")[1] # everything after the :root blocks +import re +print("literals left:", re.findall(r"(?:background|color)\s*:\s*#[0-9a-f]{3,6}", body) or "none") +PY +``` + +## Delivering the doc + +Give the user the **full absolute path**, on its own line, for every artifact you wrote. Terminal and desktop chat interfaces turn an absolute path into a clickable link, and clicking is how the user actually opens these. A bare filename, a repo-relative path, or a path in prose is not clickable and forces them to reconstruct it. + +Get the directory right, not just the name. Reviews are frequently run from a **git worktree**, and `reviews/` is typically gitignored, so the file exists only under the worktree it was written in and no git operation will ever move it. Resolve the real path rather than assuming the user shares your working directory: + +```bash +realpath reviews/REVIEW---BY-AREA.html +``` + +If the repo has a main checkout separate from your worktree, say which copy you are pointing at and note that the worktree copy dies with `git worktree remove`. Offer the `cp` into the main checkout's `reviews/` rather than leaving the user to work it out, and match whatever naming the existing files there already use. + +Name files with a slug, not just a date, so they stay distinguishable once several accumulate in one folder: `REVIEW---BY-AREA.md` (for example `REVIEW-2026-08-02-SHOPPING-LIST-SHARING-BY-AREA.md`). From f4adef194db51441b490f052597a67fff6faee22 Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:28:24 +0200 Subject: [PATCH 08/21] fix(shopping-lists): Stop sending account ids to link visitors Changes: - Null ownerId in the list DTO unless the caller is the owner - Give toItemDto the caller's access and null updatedByUserId for everyone else - Pass the resolved access through from both the shared and the owner-only services linkAccess and shareToken were already gated on ownership, but ownerId and each item's updatedByUserId were not. An anonymous caller holding only a share token received the owner's account id, and on a SHOP or EDIT list the account id of every collaborator who had touched an item. Those are stable cross-request identifiers, the same value the JWT carries as sub, so a forwarded link handed out a way to correlate two share links as belonging to one person. Nothing on the frontend reads ownerId any more, and no UI renders attribution yet, so this removes data rather than breaking a caller. --- .../service/SharedShoppingListService.java | 2 +- .../service/ShoppingListMapper.java | 27 ++++++++++++++----- .../service/ShoppingListItemService.java | 7 ++--- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/backend/src/main/java/disscount/shoppingList/service/SharedShoppingListService.java b/backend/src/main/java/disscount/shoppingList/service/SharedShoppingListService.java index c34538d9..e55001b9 100644 --- a/backend/src/main/java/disscount/shoppingList/service/SharedShoppingListService.java +++ b/backend/src/main/java/disscount/shoppingList/service/SharedShoppingListService.java @@ -69,7 +69,7 @@ public Optional updateItem( ShoppingListItem saved = shoppingListItemRepository.save(item); touchList(list); - return shoppingListMapper.toItemDto(saved); + return shoppingListMapper.toItemDto(saved, access); }); }); } diff --git a/backend/src/main/java/disscount/shoppingList/service/ShoppingListMapper.java b/backend/src/main/java/disscount/shoppingList/service/ShoppingListMapper.java index 53ac1f69..656ef09a 100644 --- a/backend/src/main/java/disscount/shoppingList/service/ShoppingListMapper.java +++ b/backend/src/main/java/disscount/shoppingList/service/ShoppingListMapper.java @@ -9,6 +9,7 @@ import disscount.shoppingListItem.dto.ShoppingListItemDto; import java.util.List; +import java.util.UUID; import java.util.stream.Collectors; /** @@ -19,16 +20,19 @@ public class ShoppingListMapper { public ShoppingListDto toDto(ShoppingList list, ListAccess access) { + boolean isOwner = access == ListAccess.OWNER; + List items = list.getItems().stream() .filter(item -> item.getDeletedAt() == null) - .map(this::toItemDto) + .map(item -> toItemDto(item, access)) .collect(Collectors.toList()); - boolean isOwner = access == ListAccess.OWNER; - return ShoppingListDto.builder() .id(list.getId()) - .ownerId(list.getOwner().getId()) + // Account ids are for the owner only. They are stable cross-request + // identifiers, and a share link can travel anywhere, so a recipient + // would otherwise be able to correlate two links as the same person. + .ownerId(isOwner ? list.getOwner().getId() : null) .title(list.getTitle()) .linkAccess(isOwner ? list.resolvedLinkAccess() : null) .shareToken(isOwner ? list.getShareToken() : null) @@ -39,7 +43,7 @@ public ShoppingListDto toDto(ShoppingList list, ListAccess access) { .build(); } - public ShoppingListItemDto toItemDto(ShoppingListItem item) { + public ShoppingListItemDto toItemDto(ShoppingListItem item, ListAccess access) { return ShoppingListItemDto.builder() .id(item.getId()) .shoppingListId(item.getShoppingList().getId()) @@ -55,7 +59,18 @@ public ShoppingListItemDto toItemDto(ShoppingListItem item) { .storePrice(item.getStorePrice()) .createdAt(item.getCreatedAt()) .updatedAt(item.getUpdatedAt()) - .updatedByUserId(item.getUpdatedByUser() != null ? item.getUpdatedByUser().getId() : null) + // Same reason as ownerId. On a SHOP or EDIT list several accounts touch + // items, so this would hand a link visitor the account id of everyone + // shopping it. Nothing renders attribution yet; version 2 adds it as a + // name, not an id. + .updatedByUserId(updatedByUserId(item, access)) .build(); } + + private UUID updatedByUserId(ShoppingListItem item, ListAccess access) { + if (access != ListAccess.OWNER || item.getUpdatedByUser() == null) { + return null; + } + return item.getUpdatedByUser().getId(); + } } diff --git a/backend/src/main/java/disscount/shoppingListItem/service/ShoppingListItemService.java b/backend/src/main/java/disscount/shoppingListItem/service/ShoppingListItemService.java index d9ffba44..e5051e19 100644 --- a/backend/src/main/java/disscount/shoppingListItem/service/ShoppingListItemService.java +++ b/backend/src/main/java/disscount/shoppingListItem/service/ShoppingListItemService.java @@ -7,6 +7,7 @@ import disscount.exceptions.BadRequestException; import disscount.exceptions.UnauthorizedException; import disscount.shoppingList.dao.ShoppingListRepository; +import disscount.shoppingList.domain.ListAccess; import disscount.shoppingList.domain.ShoppingList; import disscount.shoppingList.service.ShoppingListMapper; import disscount.shoppingListItem.dao.ShoppingListItemRepository; @@ -93,7 +94,7 @@ public ShoppingListItemDto addItemToShoppingList(UUID shoppingListId, UUID owner shoppingList.setUpdatedAt(Timestamps.nowUtc()); shoppingListRepository.save(shoppingList); - return shoppingListMapper.toItemDto(item); + return shoppingListMapper.toItemDto(item, ListAccess.OWNER); } public ShoppingListItemDto updateShoppingListItem(UUID listId, UUID itemId, UUID ownerId, ShoppingListItemRequest request) { @@ -124,7 +125,7 @@ public ShoppingListItemDto updateShoppingListItem(UUID listId, UUID itemId, UUID item.getShoppingList().setUpdatedAt(Timestamps.nowUtc()); shoppingListRepository.save(item.getShoppingList()); - return shoppingListMapper.toItemDto(item); + return shoppingListMapper.toItemDto(item, ListAccess.OWNER); } public void deleteShoppingListItem(UUID listId, UUID itemId, UUID ownerId) { @@ -147,7 +148,7 @@ public List getUserShoppingListItems(UUID ownerId) { return shoppingListItemRepository.findAllActiveItemsByUser(owner) .stream() - .map(shoppingListMapper::toItemDto) + .map(item -> shoppingListMapper.toItemDto(item, ListAccess.OWNER)) .collect(Collectors.toList()); } From 1e4c42e724d1ad82bd1a1c100fc93923dc7b34f2 Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:31:09 +0200 Subject: [PATCH 09/21] fix(shopping-lists): Let a stale token fall back to anonymous on share links Changes: - Add OptionalBearerAuthenticationFilter, which authenticates a bearer token when it decodes and lets the request through anonymously when it does not - Give /api/shared/** its own @Order(1) filter chain that uses it - Add a LinkAccess enum without OWNER and bind ShoppingListRequest to it permitAll only decides authorization. BearerTokenAuthenticationFilter still ran on every request and answered 401 for an expired or malformed token before the authorization rules were consulted, so a user whose cached token had gone stale, or who signed out in another tab, could not open a share link at all. Dropping the resource server from the shared chain would have fixed that and broken the other half, since a signed-in recipient would then be seen as anonymous and capped at VIEW however generous the link is. The optional filter is what lets both callers work. ShoppingListRequest previously accepted the full ListAccess, including OWNER. applyLinkAccess rejected it, so this was never exploitable, but the guard was the only thing preventing a caller from granting themselves share management. OWNER is now unrepresentable on the wire and Jackson rejects it before any service runs. Notes: - Verified by reading the chain: oauth2ResourceServer installs the bearer filter chain-wide, and its entry point commits the response rather than continuing. - Also drops a LocalDateTime import left dead by the earlier UTC timestamp sweep. --- .../OptionalBearerAuthenticationFilter.java | 60 +++++++++++++++++++ .../java/disscount/config/SecurityConfig.java | 32 +++++++++- .../shoppingList/domain/LinkAccess.java | 23 +++++++ .../shoppingList/dto/ShoppingListRequest.java | 5 +- .../service/ShoppingListService.java | 16 ++--- 5 files changed, 124 insertions(+), 12 deletions(-) create mode 100644 backend/src/main/java/disscount/config/OptionalBearerAuthenticationFilter.java create mode 100644 backend/src/main/java/disscount/shoppingList/domain/LinkAccess.java diff --git a/backend/src/main/java/disscount/config/OptionalBearerAuthenticationFilter.java b/backend/src/main/java/disscount/config/OptionalBearerAuthenticationFilter.java new file mode 100644 index 00000000..9fdfceb5 --- /dev/null +++ b/backend/src/main/java/disscount/config/OptionalBearerAuthenticationFilter.java @@ -0,0 +1,60 @@ +package disscount.config; + +import jakarta.servlet.FilterChain; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import lombok.RequiredArgsConstructor; +import org.springframework.http.HttpHeaders; +import org.springframework.lang.NonNull; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.oauth2.jwt.Jwt; +import org.springframework.security.oauth2.jwt.JwtDecoder; +import org.springframework.security.oauth2.jwt.JwtException; +import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter; +import org.springframework.stereotype.Component; +import org.springframework.web.filter.OncePerRequestFilter; + +import java.io.IOException; + +/** + * Authenticates a bearer token when one is present and usable, and lets the request through + * anonymously when it is not. + * + *

This exists because a share link has to work for both. The standard + * {@code BearerTokenAuthenticationFilter} rejects an expired or malformed token with 401 + * before authorization rules are consulted, so a permitAll endpoint is not actually reachable + * by a caller whose cached token has gone stale. Dropping the resource server from the shared + * chain instead would fix that but break the other half: a signed-in recipient would be seen + * as anonymous and capped at VIEW however generous the link is. + */ +@Component +@RequiredArgsConstructor +public class OptionalBearerAuthenticationFilter extends OncePerRequestFilter { + + private static final String BEARER_PREFIX = "Bearer "; + + private final JwtDecoder jwtDecoder; + private final JwtAuthenticationConverter converter = new JwtAuthenticationConverter(); + + @Override + protected void doFilterInternal( + @NonNull HttpServletRequest request, + @NonNull HttpServletResponse response, + @NonNull FilterChain chain + ) throws ServletException, IOException { + String header = request.getHeader(HttpHeaders.AUTHORIZATION); + + if (header != null && header.startsWith(BEARER_PREFIX)) { + try { + Jwt jwt = jwtDecoder.decode(header.substring(BEARER_PREFIX.length())); + SecurityContextHolder.getContext().setAuthentication(converter.convert(jwt)); + } catch (JwtException ignored) { + // Expired, malformed or issued elsewhere. The caller keeps whatever the link + // grants an anonymous visitor, which the access resolver caps at VIEW. + } + } + + chain.doFilter(request, response); + } +} diff --git a/backend/src/main/java/disscount/config/SecurityConfig.java b/backend/src/main/java/disscount/config/SecurityConfig.java index 68196949..5b97d81a 100644 --- a/backend/src/main/java/disscount/config/SecurityConfig.java +++ b/backend/src/main/java/disscount/config/SecurityConfig.java @@ -5,6 +5,7 @@ import org.springframework.beans.factory.annotation.Value; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.core.annotation.Order; import org.springframework.http.HttpMethod; import org.springframework.security.config.Customizer; import org.springframework.security.config.annotation.web.builders.HttpSecurity; @@ -16,6 +17,7 @@ import org.springframework.security.oauth2.jwt.NimbusJwtDecoder; import org.springframework.security.oauth2.server.resource.web.BearerTokenAuthenticationFilter; import org.springframework.security.web.SecurityFilterChain; +import org.springframework.security.web.authentication.AnonymousAuthenticationFilter; @Configuration @EnableWebSecurity @@ -39,16 +41,40 @@ public JwtDecoder jwtDecoder( return decoder; } + /** + * Shared lists get their own chain because they are the one place where a bearer token is + * optional. Authorization happens on the share token plus ShoppingListAccessService, and + * the caller may legitimately be anonymous, so a token that fails to decode must degrade + * to anonymous rather than 401. permitAll on the main chain cannot express that: its + * bearer filter rejects a stale token before authorization is ever consulted. + */ @Bean + @Order(1) + public SecurityFilterChain sharedShoppingListChain( + HttpSecurity http, + OptionalBearerAuthenticationFilter optionalBearerAuthenticationFilter + ) throws Exception { + http + .securityMatcher("/api/shared/**") + .csrf(csrf -> csrf.disable()) + .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) + .authorizeHttpRequests(authz -> authz.anyRequest().permitAll()) + // Both before the anonymous filter, in registration order, so the token is + // resolved first and provisioning sees the authentication it produced. + .addFilterBefore(optionalBearerAuthenticationFilter, AnonymousAuthenticationFilter.class) + .addFilterBefore(userProvisioningFilter, AnonymousAuthenticationFilter.class); + + return http.build(); + } + + @Bean + @Order(2) public SecurityFilterChain filterChain(HttpSecurity http) throws Exception { http .csrf(csrf -> csrf.disable()) .sessionManagement(session -> session.sessionCreationPolicy(SessionCreationPolicy.STATELESS)) .authorizeHttpRequests(authz -> authz .requestMatchers(HttpMethod.POST, "/api/contact").permitAll() - // Shared lists authorize on the share token plus ShoppingListAccessService, - // not on the filter chain, because the caller may legitimately be anonymous. - .requestMatchers("/api/shared/**").permitAll() .requestMatchers( "/actuator/health", "/v3/api-docs/**", diff --git a/backend/src/main/java/disscount/shoppingList/domain/LinkAccess.java b/backend/src/main/java/disscount/shoppingList/domain/LinkAccess.java new file mode 100644 index 00000000..b722947f --- /dev/null +++ b/backend/src/main/java/disscount/shoppingList/domain/LinkAccess.java @@ -0,0 +1,23 @@ +package disscount.shoppingList.domain; + +/** + * The levels a share link can actually be set to, which is every {@link ListAccess} value + * except OWNER. + * + *

This is a separate enum rather than a validated {@code ListAccess} so that OWNER is not + * expressible on the wire at all. Jackson rejects it during deserialization, before any + * service code runs, which means the guard in {@code ShoppingListService.applyLinkAccess} is + * no longer the only thing standing between a typo and a caller granting themselves the + * ability to manage sharing. + */ +public enum LinkAccess { + + NONE, + VIEW, + SHOP, + EDIT; + + public ListAccess toListAccess() { + return ListAccess.valueOf(name()); + } +} diff --git a/backend/src/main/java/disscount/shoppingList/dto/ShoppingListRequest.java b/backend/src/main/java/disscount/shoppingList/dto/ShoppingListRequest.java index 24653f19..eff3bbbf 100644 --- a/backend/src/main/java/disscount/shoppingList/dto/ShoppingListRequest.java +++ b/backend/src/main/java/disscount/shoppingList/dto/ShoppingListRequest.java @@ -3,7 +3,7 @@ import jakarta.validation.constraints.NotBlank; import lombok.Data; -import disscount.shoppingList.domain.ListAccess; +import disscount.shoppingList.domain.LinkAccess; @Data public class ShoppingListRequest { @@ -13,5 +13,6 @@ public class ShoppingListRequest { // Owner-only, and ignored on create: sharing is turned on from an existing list, // because there is no id to bind a token to until the list has been saved. - private ListAccess linkAccess; + // LinkAccess rather than ListAccess, so OWNER cannot be sent at all. + private LinkAccess linkAccess; } diff --git a/backend/src/main/java/disscount/shoppingList/service/ShoppingListService.java b/backend/src/main/java/disscount/shoppingList/service/ShoppingListService.java index 32a26618..d73c8685 100644 --- a/backend/src/main/java/disscount/shoppingList/service/ShoppingListService.java +++ b/backend/src/main/java/disscount/shoppingList/service/ShoppingListService.java @@ -7,6 +7,7 @@ import disscount.exceptions.BadRequestException; import disscount.exceptions.UnauthorizedException; import disscount.shoppingList.dao.ShoppingListRepository; +import disscount.shoppingList.domain.LinkAccess; import disscount.shoppingList.domain.ListAccess; import disscount.shoppingList.domain.ShoppingList; import disscount.shoppingList.dto.ShoppingListDto; @@ -15,7 +16,6 @@ import disscount.user.domain.User; import disscount.util.Timestamps; -import java.time.LocalDateTime; import java.util.List; import java.util.Optional; import java.util.UUID; @@ -101,21 +101,23 @@ public void deleteShoppingList(UUID listId, UUID ownerId) { * revoke. Merely changing the level leaves the token alone, since the people already * holding the link are meant to keep working at the new level. */ - private void applyLinkAccess(ShoppingList list, ListAccess requested) { - if (requested == null || requested == list.resolvedLinkAccess()) { + private void applyLinkAccess(ShoppingList list, LinkAccess requested) { + if (requested == null) { return; } - if (requested == ListAccess.OWNER) { - throw new BadRequestException("OWNER is not a link access level"); + + ListAccess next = requested.toListAccess(); + if (next == list.resolvedLinkAccess()) { + return; } - if (requested == ListAccess.NONE) { + if (next == ListAccess.NONE) { list.setLinkAccess(null); list.setShareToken(null); return; } - list.setLinkAccess(requested); + list.setLinkAccess(next); if (list.getShareToken() == null) { list.setShareToken(UUID.randomUUID()); } From 14eec4a058cbf347b143d3e93756f476f5b2c4cf Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:34:03 +0200 Subject: [PATCH 10/21] fix(shopping-lists): Redact the share token from telemetry and referrers Changes: - Add scrubShareToken, rewriting /s/ and /api/shared/ to a placeholder - Wire it into beforeSend, beforeSendTransaction and beforeBreadcrumb on the client - Wire it into the server config, which sees the token through the preview fetch - Send Referrer-Policy: no-referrer on /s/ The token is a bearer capability: whoever holds the URL can read the list, and can tick items off or edit them depending on the link level. Sentry attaches the page URL to every event, records fetch breadcrumbs carrying the request URL, and replays navigations, none of which sendDefaultPii: false covers, since that gates IP, cookies and headers rather than URLs. Any error on a shared page, or a one-in-ten sampled clean session, shipped a working link to Sentry. Notes: - Also documents why the /shopping-lists X-Robots-Tag rule is inert but kept: robots.ts already disallows the prefix, so a compliant crawler never reads it. - Proxy access logs still record the full path. That is a Dokploy-side log format change, recorded in the docs batch rather than fixed here. --- frontend/next.config.ts | 11 ++++- frontend/sentry.server.config.ts | 7 +++ frontend/src/instrumentation-client.ts | 16 +++++++ frontend/src/lib/sentry/scrub-share-token.ts | 46 ++++++++++++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 frontend/src/lib/sentry/scrub-share-token.ts diff --git a/frontend/next.config.ts b/frontend/next.config.ts index 19e293a7..d770eb32 100644 --- a/frontend/next.config.ts +++ b/frontend/next.config.ts @@ -78,9 +78,18 @@ const nextConfig: NextConfig = { // so it would never see the directive. A shared link only leaks by being pasted // somewhere crawlable, which is exactly the case this covers. source: "/s/:path*", - headers: [{ key: "X-Robots-Tag", value: "noindex, nofollow" }], + headers: [ + { key: "X-Robots-Tag", value: "noindex, nofollow" }, + // The token is in the path, so the default strict-origin-when-cross-origin + // would still hand the whole URL to any same-origin subresource and the + // origin to third parties. Nothing on this page needs a referrer. + { key: "Referrer-Policy", value: "no-referrer" }, + ], }, { + // Belt and braces: robots.ts already disallows this prefix, so a compliant + // crawler never fetches the page and never reads this header. It is here for + // one that ignores robots.txt. source: "/shopping-lists/:path*", headers: [{ key: "X-Robots-Tag", value: "noindex, nofollow" }], }, diff --git a/frontend/sentry.server.config.ts b/frontend/sentry.server.config.ts index 6912e5aa..e8abc32b 100644 --- a/frontend/sentry.server.config.ts +++ b/frontend/sentry.server.config.ts @@ -2,9 +2,16 @@ // client); read at runtime so it can be set per-environment and no-ops when unset. import * as Sentry from "@sentry/nextjs"; +import { scrubEventUrls } from "@/lib/sentry/scrub-share-token"; + Sentry.init({ dsn: process.env.NEXT_PUBLIC_SENTRY_DSN, + // getSharedListPreview fetches /api/shared/ server-side for the link preview, + // so the token reaches the server traces too. + beforeSend: scrubEventUrls, + beforeSendTransaction: scrubEventUrls, + tracesSampleRate: process.env.NODE_ENV === "production" ? 0.1 : 1.0, enableLogs: true, diff --git a/frontend/src/instrumentation-client.ts b/frontend/src/instrumentation-client.ts index cca44342..d623dd01 100644 --- a/frontend/src/instrumentation-client.ts +++ b/frontend/src/instrumentation-client.ts @@ -2,11 +2,27 @@ // redeploy) and no-ops cleanly when unset. import * as Sentry from "@sentry/nextjs"; +import { + scrubCrumbData, + scrubEventUrls, + scrubShareToken, +} from "@/lib/sentry/scrub-share-token"; + Sentry.init({ dsn: process.env.NEXT_PUBLIC_SENTRY_DSN, + // Replay masks text by default but not URLs, so a navigation to a shared list would + // otherwise carry a working capability token into the recording. integrations: [Sentry.replayIntegration()], + beforeSend: scrubEventUrls, + beforeSendTransaction: scrubEventUrls, + beforeBreadcrumb(breadcrumb) { + breadcrumb.message = scrubShareToken(breadcrumb.message); + if (breadcrumb.data) breadcrumb.data = scrubCrumbData(breadcrumb.data); + return breadcrumb; + }, + // 100% of traces in dev, 10% in production tracesSampleRate: process.env.NODE_ENV === "production" ? 0.1 : 1.0, diff --git a/frontend/src/lib/sentry/scrub-share-token.ts b/frontend/src/lib/sentry/scrub-share-token.ts new file mode 100644 index 00000000..5af82b35 --- /dev/null +++ b/frontend/src/lib/sentry/scrub-share-token.ts @@ -0,0 +1,46 @@ +/** + * A share token is a bearer capability: anyone holding the URL can read, and depending on + * the link level tick items off or edit them. It travels in the path of `/s/` and + * `/api/shared/`, and Sentry attaches the page URL to every event, records fetch + * breadcrumbs with the request URL, and replays navigations. `sendDefaultPii: false` does + * not cover any of that, because it gates IP, cookies and headers rather than URLs. + * + * So the token is redacted from anything on its way out. Losing the exact value costs + * nothing for debugging: the path shape is what identifies the route. + */ +const TOKEN_PATHS = /\/(s|api\/shared)\/[^/?#]+/g; + +export function scrubShareToken(value: T): T { + if (typeof value !== "string") return value; + return value.replace(TOKEN_PATHS, "/$1/[token]") as T; +} + +/** Rewrites the string-valued URL fields Sentry puts on an event or breadcrumb. */ +export function scrubEventUrls< + T extends { + request?: { url?: string } | undefined; + breadcrumbs?: { data?: Record }[] | undefined; + }, +>(event: T): T { + if (event.request?.url) { + event.request.url = scrubShareToken(event.request.url); + } + + for (const breadcrumb of event.breadcrumbs ?? []) { + if (breadcrumb.data) breadcrumb.data = scrubCrumbData(breadcrumb.data); + } + + return event; +} + +export function scrubCrumbData( + data: Record, +): Record { + const scrubbed: Record = {}; + + for (const [key, value] of Object.entries(data)) { + scrubbed[key] = scrubShareToken(value); + } + + return scrubbed; +} From 80313ddeafc30e4f3645999c1e8b2f34fd95448c Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:37:35 +0200 Subject: [PATCH 11/21] fix(shopping-lists): Key the offline cache by identity and stop purging on every load Changes: - Add cache-identity.ts, which scopes the IndexedDB entry to the signed-in account - Resolve the storage key per call, so an identity change lands on the next read or write - Purge only when the identity actually changes, not whenever there is no session - Delete the pages and cijene-api service worker buckets as part of the purge - Bump CACHE_BUSTER to "3", since the old shared entry is now orphaned Two bugs, one cause. The persisted cache was a single browser-wide blob shared by every account on a device, with a destructive purge as the only defence. And that purge ran from an effect whose condition was "there is no session", which is true on every page load for a visitor who never logs in, not just at the moment they log out. So an anonymous visitor who ticked items off a shared list while offline lost the queue on the next boot: no error, nothing to replay, and removeClient had already deleted the snapshot that would have recovered it. Cache Storage had the mirror of the same problem. Nothing in the app called caches.delete(), so the pages bucket kept server-rendered documents and cijene-api kept one entry per product looked at. Each product is public on its own, but the set of them is the contents of whichever list was open. Notes: - The identity is mirrored in localStorage because the persister must choose a key synchronously at boot, before the session resolves. --- frontend/src/context/user-context.tsx | 24 ++++++++++- frontend/src/lib/offline/cache-identity.ts | 49 ++++++++++++++++++++++ frontend/src/lib/offline/persister.ts | 13 ++++-- frontend/src/lib/offline/purge.ts | 26 ++++++++++++ 4 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 frontend/src/lib/offline/cache-identity.ts diff --git a/frontend/src/context/user-context.tsx b/frontend/src/context/user-context.tsx index 1537ec5f..51250ff5 100644 --- a/frontend/src/context/user-context.tsx +++ b/frontend/src/context/user-context.tsx @@ -6,6 +6,7 @@ import { useEffect, useState, useCallback, + useRef, ReactNode, } from "react"; import { usePathname, useRouter } from "next/navigation"; @@ -14,6 +15,10 @@ import { useQueryClient } from "@tanstack/react-query"; import { authClient, useSession } from "@/lib/auth/client"; import { clearAuthToken, resetAuthToken } from "@/lib/api/api-base"; import { purgeOfflineCache } from "@/lib/offline/purge"; +import { + getCacheIdentity, + setCacheIdentity, +} from "@/lib/offline/cache-identity"; import { userService, preferencesService } from "@/lib/api"; import { UserDto, PinnedStoreDto, PinnedPlaceDto } from "@/lib/api/types"; import { isProtectedRoute } from "@/constants/protected-routes"; @@ -48,6 +53,11 @@ export function UserProvider({ children }: IUserProviderProps) { const [user, setUser] = useState(null); const [isLoading, setIsLoading] = useState(true); const [hasResolvedAuth, setHasResolvedAuth] = useState(false); + // Seeded from the persisted value so a reload as the same account is not read as a + // change of identity, which would purge the cache it just restored. + const cacheIdentityRef = useRef( + getCacheIdentity() === "anon" ? null : getCacheIdentity(), + ); const queryClient = useQueryClient(); const router = useRouter(); @@ -86,12 +96,22 @@ export function UserProvider({ children }: IUserProviderProps) { useEffect(() => { if (sessionPending) return; + const identity = session?.user?.id ?? null; + + // Purge on a change of identity, not on every anonymous load. This effect runs on + // mount, so purging whenever there is no session wiped the cache and the queued + // write replay on every single page load for a visitor who is not logged in, which + // silently dropped anything they had ticked off while offline. + if (identity !== cacheIdentityRef.current) { + void purgeOfflineCache(queryClient); + setCacheIdentity(identity); + cacheIdentityRef.current = identity; + } + if (session?.user) { refreshUser(); } else { - // Wipe the cache so a previous user never lingers on a shared device. clearAuthToken(); - void purgeOfflineCache(queryClient); setUser(null); setIsLoading(false); setHasResolvedAuth(true); diff --git a/frontend/src/lib/offline/cache-identity.ts b/frontend/src/lib/offline/cache-identity.ts new file mode 100644 index 00000000..9d3c6e82 --- /dev/null +++ b/frontend/src/lib/offline/cache-identity.ts @@ -0,0 +1,49 @@ +/** + * Which account the persisted query cache belongs to. + * + * The offline cache is one browser-wide IndexedDB entry, so without this every account on + * a device reads and writes the same blob, and the only defence is a destructive purge. + * That was tolerable while everything cached was your own; shared lists make it someone + * else's data, so the store is now keyed by identity and each account gets its own entry. + * + * Mirrored in localStorage because the persister has to choose a key synchronously at boot, + * before the session has resolved. + */ +const IDENTITY_STORAGE_KEY = "disscount-cache-identity"; +const ANONYMOUS = "anon"; + +let currentIdentity: string | null = null; + +function readStoredIdentity(): string { + if (typeof window === "undefined") return ANONYMOUS; + + try { + return window.localStorage.getItem(IDENTITY_STORAGE_KEY) ?? ANONYMOUS; + } catch { + // Private mode or blocked storage. Everyone shares the anonymous bucket, which is + // the pre-existing behaviour rather than a regression. + return ANONYMOUS; + } +} + +export function getCacheIdentity(): string { + currentIdentity ??= readStoredIdentity(); + return currentIdentity; +} + +export function setCacheIdentity(userId: string | null): void { + currentIdentity = userId ?? ANONYMOUS; + + if (typeof window === "undefined") return; + + try { + window.localStorage.setItem(IDENTITY_STORAGE_KEY, currentIdentity); + } catch { + // Nothing to do: the in-memory value still scopes this session's writes. + } +} + +/** Appends the current identity, so one account's cache is unreadable under another. */ +export function scopedCacheKey(baseKey: string): string { + return `${baseKey}:${getCacheIdentity()}`; +} diff --git a/frontend/src/lib/offline/persister.ts b/frontend/src/lib/offline/persister.ts index d6cce691..1b8006e7 100644 --- a/frontend/src/lib/offline/persister.ts +++ b/frontend/src/lib/offline/persister.ts @@ -6,6 +6,7 @@ import { import type { PersistQueryClientOptions } from "@tanstack/react-query-persist-client"; import { get, set, del } from "idb-keyval"; +import { scopedCacheKey } from "@/lib/offline/cache-identity"; import { shouldPersistQuery } from "@/lib/offline/cached-query-keys"; import { shouldPersistMutation } from "@/lib/offline/offline-mutation-keys"; @@ -15,15 +16,19 @@ const IDB_CACHE_KEY = "disscount-react-query-cache"; // "2": ShoppingListDto dropped isPublic and gained linkAccess, shareToken and myAccess. // A restored pre-change list has no myAccess, which every capability check would read // as no access at all. -const CACHE_BUSTER = "2"; +// "3": the entry is now keyed per identity, so the old shared blob is orphaned. +const CACHE_BUSTER = "3"; export const OFFLINE_CACHE_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; // 7 days // IndexedDB over localStorage: larger, and safer for cached application data. +// The key is resolved per call rather than once, so an identity change takes effect on +// the next read or write without rebuilding the persister. const indexedDbStorage = { - getItem: async (key: string) => (await get(key)) ?? null, - setItem: (key: string, value: string) => set(key, value), - removeItem: (key: string) => del(key), + getItem: async (key: string) => + (await get(scopedCacheKey(key))) ?? null, + setItem: (key: string, value: string) => set(scopedCacheKey(key), value), + removeItem: (key: string) => del(scopedCacheKey(key)), }; export const offlinePersister = createAsyncStoragePersister({ diff --git a/frontend/src/lib/offline/purge.ts b/frontend/src/lib/offline/purge.ts index c642a9ef..60a646e2 100644 --- a/frontend/src/lib/offline/purge.ts +++ b/frontend/src/lib/offline/purge.ts @@ -5,11 +5,36 @@ import { offlinePersister } from "@/lib/offline/persister"; // Public data is identical logged in or out, so it survives logout. const PUBLIC_QUERY_ROOT = "cijene"; +// Service worker buckets that can hold data belonging to whoever was just here. The +// pages bucket keeps server-rendered documents keyed by URL alone, and cijene-api keeps +// one entry per product looked at, so on a shared list the set of cached EANs is the +// list's contents even though each product is public on its own. +const SCOPED_CACHE_NAMES = ["pages", "cijene-api"]; + // Everything outside the public root is user-specific and gets purged. function isUserSpecific(query: Query): boolean { return query.queryKey[0] !== PUBLIC_QUERY_ROOT; } +/** + * Cache Storage is not identity-scoped and nothing else in the app ever deletes from it, + * so without this a shared list's documents and product lookups outlive the session that + * fetched them. + */ +async function purgeServiceWorkerCaches() { + if (typeof caches === "undefined") return; + + const names = await caches.keys(); + + await Promise.all( + names + .filter((name) => + SCOPED_CACHE_NAMES.some((scoped) => name.includes(scoped)), + ) + .map((name) => caches.delete(name)), + ); +} + export async function purgeOfflineCache(queryClient: QueryClient) { // Cancel in-flight work first so a late-resolving request can't repopulate what we clear. await queryClient.cancelQueries({ predicate: isUserSpecific }); @@ -20,6 +45,7 @@ export async function purgeOfflineCache(queryClient: QueryClient) { try { // Wipe the snapshot so no authed data lingers; public data re-persists on next save. await offlinePersister.removeClient(); + await purgeServiceWorkerCaches(); } catch (error) { // Never let a failed IndexedDB purge block logout / auth-loss handling. console.error("Failed to clear the persisted offline cache", error); From 7f4ef14ea36b07fb7d4ef04b507588f294c543bb Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:37:35 +0200 Subject: [PATCH 12/21] feat(shopping-lists): Make shared lists work offline Changes: - Persist the sharedShoppingList query root - Serve /s/ documents NetworkFirst instead of NetworkOnly, with a one-day expiry - Correct the byToken comment, whose stated premise was wrong A shared list had its writes queued for offline replay but its reads kept out of the persisted cache, so a reload with no signal found nothing on disk, the query failed, and the page rendered "the owner stopped sharing this list" for a link that was perfectly valid. NetworkOnly on the document compounded it: the offline navigation fell through to the /offline page, so the queued writes were not even reachable to look at. Both were deliberate privacy choices when a persisted shared list meant someone else's data in one browser-wide blob. Now that the cache is keyed by identity and purged when that identity changes, the trade no longer has to be paid. Notes: - The byToken comment claimed purgeOfflineCache only runs on a logout transition. It never did, and reasoning from that is what produced the original design. --- frontend/src/app/sw.ts | 19 +++++++++++++++---- frontend/src/lib/api/shopping-lists/keys.ts | 8 ++++---- frontend/src/lib/offline/cached-query-keys.ts | 4 ++++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/frontend/src/app/sw.ts b/frontend/src/app/sw.ts index 73d69e52..9635b3e7 100644 --- a/frontend/src/app/sw.ts +++ b/frontend/src/app/sw.ts @@ -7,6 +7,7 @@ import type { import { CacheableResponsePlugin, ExpirationPlugin, + NetworkFirst, NetworkOnly, Serwist, StaleWhileRevalidate, @@ -44,13 +45,23 @@ const runtimeCaching: RuntimeCaching[] = [ sameOrigin && url.pathname.startsWith("/api/"), handler: new NetworkOnly(), }, - // Shared lists server-render someone else's list title for the link preview, and - // defaultCache would keep that document for 24 days keyed by URL alone, with no notion - // of who asked. Must stay above defaultCache, which is matched in order. + // Shared lists server-render someone else's list title for the link preview, so the + // document is not public data. NetworkOnly kept it off disk but sent every offline + // reload to the /offline fallback, which made the offline write queue unreachable in + // exactly the shop-with-no-signal case it exists for. NetworkFirst with a short life + // plus purgeOfflineCache deleting this bucket on a change of identity is the trade. + // Must stay above defaultCache, which is matched in order. { matcher: ({ url, sameOrigin }) => sameOrigin && url.pathname.startsWith("/s/"), - handler: new NetworkOnly(), + handler: new NetworkFirst({ + cacheName: "pages", + networkTimeoutSeconds: 5, + plugins: [ + new CacheableResponsePlugin({ statuses: [0, 200] }), + new ExpirationPlugin({ maxEntries: 20, maxAgeSeconds: 24 * 60 * 60 }), + ], + }), }, ...defaultCache, ]; diff --git a/frontend/src/lib/api/shopping-lists/keys.ts b/frontend/src/lib/api/shopping-lists/keys.ts index ca9b6c9e..2c99b9e2 100644 --- a/frontend/src/lib/api/shopping-lists/keys.ts +++ b/frontend/src/lib/api/shopping-lists/keys.ts @@ -11,10 +11,10 @@ export const SHOPPING_LIST_QUERY_KEYS = { myItems: ["shoppingListItems", "me"] as const, /** - * Deliberately NOT under the `shoppingLists` root, so it is never persisted. The - * IndexedDB cache is a single browser-wide store and purgeOfflineCache only runs on a - * logout transition, so a visitor who never logs in never purges: a persisted shared - * list would sit on a stranger's disk for the seven-day cache lifetime. + * Its own root rather than a branch of `shoppingLists`, because a list reached by token + * is not one of yours: it must not appear in the owner-scoped invalidations, and it has + * its own persistence and purge story. See lib/offline/cache-identity.ts, which is what + * makes persisting someone else's list acceptable. */ byToken: (token: string) => ["sharedShoppingList", token] as const, }; diff --git a/frontend/src/lib/offline/cached-query-keys.ts b/frontend/src/lib/offline/cached-query-keys.ts index 8c02d110..2426cb44 100644 --- a/frontend/src/lib/offline/cached-query-keys.ts +++ b/frontend/src/lib/offline/cached-query-keys.ts @@ -12,6 +12,10 @@ const PERSISTED_QUERY_KEY_PREFIXES = [ "pinnedStores", "pinnedPlaces", "users", // current user profile (["users", "me"]) + // Someone else's list, reached by share token. Safe to persist now that the store is + // keyed per identity and purged when that identity changes; before that it would have + // sat in one browser-wide blob for the full seven days. + "sharedShoppingList", // TODO(offline): add /spending, /updates and /map keys when those ship. ] as const; From e2186ddd63dee5a5da0032a15c26c2447a92d0b5 Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:42:42 +0200 Subject: [PATCH 13/21] fix(shopping-lists): Roll back one item, and own the optimism in onMutate Changes: - Add optimistic-items.ts: patch, remove and restore a single item in the cache - Move cancel, snapshot and setQueryData into onMutate on all four item mutations - Restore only the affected item on error, at its original index - Invalidate onSettled rather than onSuccess - Give the shared replay handler the error, and only blame access loss on 403 or 404 Three bugs with one cause. The optimistic write lived in the event handler, so React Query did not own it: a write queued offline and replayed after a reload applied no optimistic state and had nothing to roll back, which is why the offline toast had to be bolted on in the first place. The rollback restored a whole-list snapshot. Tick item A, tick item B a moment later, and if A fails after B has already succeeded, restoring the pre-A list also un-ticks B, which the server has recorded as bought. Nothing refetched to correct it, because the owned path only invalidated on success. And the replay toast claimed lost access for every failure, so a collaborator whose request hit a timeout was told the owner had revoked their link. It now says that only for 403 and 404, which also covers a replayed delete of an already-deleted item. --- .../hooks/use-shopping-list-item-mutations.ts | 118 +++--------------- frontend/src/lib/api/shopping-lists/hooks.ts | 104 +++++++++++++-- .../api/shopping-lists/optimistic-items.ts | 95 ++++++++++++++ frontend/src/lib/offline/offline-mutations.ts | 34 +++-- 4 files changed, 230 insertions(+), 121 deletions(-) create mode 100644 frontend/src/lib/api/shopping-lists/optimistic-items.ts diff --git a/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-item-mutations.ts b/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-item-mutations.ts index b2ba268b..8fe9803e 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-item-mutations.ts +++ b/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-item-mutations.ts @@ -39,89 +39,37 @@ export function useShoppingListItemMutations( }, ) => { const shoppingList = queryClient.getQueryData(queryKey); - const item = shoppingList?.items?.find((i) => i.id === itemId); - if (!item) return; - - // Validate amount - if (updatedItem.amount < 1) return; - - // Optimistic update - await queryClient.cancelQueries({ queryKey }); - const previousData = queryClient.getQueryData(queryKey); + if (!item || updatedItem.amount < 1) return; - queryClient.setQueryData(queryKey, (old) => { - if (!old) return old; - return { - ...old, - items: old.items?.map((i) => { - if (i.id === itemId) { - const updated = { ...i, ...updatedItem }; - // If checking the item, include the current average price - if (updatedItem.isChecked) { - const currentAvgPrice = averagePrices[i.id]; - if (currentAvgPrice !== undefined) { - updated.avgPrice = currentAvgPrice; - } - } - return updated; - } - return i; - }), - }; - }); + // Prices are captured at the moment of ticking, so they have to be resolved here + // where the component's price maps live, not inside the mutation. They travel in the + // request, which is also what the optimistic patch applies. + const data = { ...item, ...updatedItem }; - // Prepare update data - const updateData = { - ...item, - ...updatedItem, - }; - - // If checking the item, include the current average price and store price if (updatedItem.isChecked) { - const currentAvgPrice = averagePrices[item.id]; - if (currentAvgPrice !== undefined) { - updateData.avgPrice = currentAvgPrice; - } + const avgPrice = averagePrices[item.id]; + if (avgPrice !== undefined) data.avgPrice = avgPrice; - // Include the store price from the selected store - if ( - updatedItem.chainCode && - storePrices[item.id]?.[updatedItem.chainCode] - ) { - updateData.storePrice = storePrices[item.id][updatedItem.chainCode]; - } - } - - function rollback() { - if (previousData) { - queryClient.setQueryData(queryKey, previousData); - } + const storePrice = + updatedItem.chainCode && storePrices[item.id]?.[updatedItem.chainCode]; + if (storePrice) data.storePrice = storePrice; } if (shareToken) { - updateSharedItemMutation.mutate( - { token: shareToken, itemId, data: updateData }, - // No toast here: the shared mutation's offline defaults already carry one, and it - // is the only handler that survives a replay after a reload. - { onError: rollback }, - ); + // No toast: the offline defaults carry one, and they are the only handler that + // survives a replay after a reload. + updateSharedItemMutation.mutate({ token: shareToken, itemId, data }); return; } updateItemMutation.mutate( + { listId, itemId, data }, { - listId, - itemId, - data: updateData, - }, - { - onError: (error: Error) => { - rollback(); + onError: (error: Error) => toast.error( error.message || "Greška pri ažuriranju stavke. Pokušaj ponovno.", - ); - }, + ), }, ); }; @@ -129,29 +77,10 @@ export function useShoppingListItemMutations( const handleDeleteItem = async (itemId: string) => { setDeletingItemId(itemId); - // Optimistic update - await queryClient.cancelQueries({ queryKey }); - const previousData = queryClient.getQueryData(queryKey); - - queryClient.setQueryData(queryKey, (old) => { - if (!old) return old; - return { - ...old, - items: old.items?.filter((i) => i.id !== itemId), - }; - }); - - function rollback() { - if (previousData) { - queryClient.setQueryData(queryKey, previousData); - } - } - if (shareToken) { deleteSharedItemMutation.mutate( { token: shareToken, itemId }, { - onError: rollback, onSuccess: () => toast.success("Stavka je uspješno obrisana!"), onSettled: () => setDeletingItemId(null), }, @@ -159,22 +88,15 @@ export function useShoppingListItemMutations( return; } - // Delete the item deleteItemMutation.mutate( { listId, itemId }, { - onError: (error: Error) => { - rollback(); + onError: (error: Error) => toast.error( error.message || "Greška pri brisanju stavke. Pokušaj ponovno.", - ); - }, - onSuccess: () => { - toast.success("Stavka je uspješno obrisana!"); - }, - onSettled: () => { - setDeletingItemId(null); - }, + ), + onSuccess: () => toast.success("Stavka je uspješno obrisana!"), + onSettled: () => setDeletingItemId(null), }, ); }; diff --git a/frontend/src/lib/api/shopping-lists/hooks.ts b/frontend/src/lib/api/shopping-lists/hooks.ts index 2f75b458..198a6c90 100644 --- a/frontend/src/lib/api/shopping-lists/hooks.ts +++ b/frontend/src/lib/api/shopping-lists/hooks.ts @@ -1,6 +1,12 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { OFFLINE_MUTATION_KEYS } from "@/lib/offline/offline-mutation-keys"; import { SHOPPING_LIST_QUERY_KEYS } from "@/lib/api/shopping-lists/keys"; +import { + patchItemOptimistically, + removeItemOptimistically, + restoreItem, + type IItemRollback, +} from "@/lib/api/shopping-lists/optimistic-items"; import { ShoppingListRequest, ShoppingListDto, @@ -62,10 +68,18 @@ export function useUpdateShoppingList() { >({ mutationKey: OFFLINE_MUTATION_KEYS.shoppingListUpdate, mutationFn: ({ id, data }) => updateShoppingList(id, data), - onSuccess: () => - queryClient.invalidateQueries({ - queryKey: SHOPPING_LIST_QUERY_KEYS.all, - }), + onSuccess: (_result, { data }) => { + queryClient.invalidateQueries({ queryKey: SHOPPING_LIST_QUERY_KEYS.all }); + + // Turning sharing off kills the token server-side, but a copy of the list read + // through it can sit in this browser's cache for the whole staleTime. Drop it so + // revoking takes effect here immediately too. + if (data.linkAccess === "NONE") { + queryClient.removeQueries({ + queryKey: SHOPPING_LIST_QUERY_KEYS.sharedRoot, + }); + } + }, }); } @@ -105,25 +119,56 @@ export function useAddItemToShoppingList() { } export function useUpdateShoppingListItem() { + const queryClient = useQueryClient(); const invalidate = useInvalidateListsAndItems(); + return useMutation< ShoppingListItemDto, Error, - { listId: string; itemId: string; data: ShoppingListItemRequest } + { listId: string; itemId: string; data: ShoppingListItemRequest }, + IItemRollback | undefined >({ mutationKey: OFFLINE_MUTATION_KEYS.shoppingListItemUpdate, mutationFn: ({ listId, itemId, data }) => updateShoppingListItem(listId, itemId, data), - onSuccess: invalidate, + // In onMutate rather than at the call site so a write restored from disk and + // replayed after a reload still applies its optimistic state: React Query only + // re-runs the optimism it owns. + onMutate: ({ listId, itemId, data }) => + patchItemOptimistically( + queryClient, + SHOPPING_LIST_QUERY_KEYS.byId(listId), + itemId, + data, + ), + onError: (_error, { listId }, rollback) => + restoreItem(queryClient, SHOPPING_LIST_QUERY_KEYS.byId(listId), rollback), + // onSettled, not onSuccess: a rolled-back cache has to reconcile with the server too. + onSettled: invalidate, }); } export function useDeleteShoppingListItem() { + const queryClient = useQueryClient(); const invalidate = useInvalidateListsAndItems(); - return useMutation({ + + return useMutation< + void, + Error, + { listId: string; itemId: string }, + IItemRollback | undefined + >({ mutationKey: OFFLINE_MUTATION_KEYS.shoppingListItemDelete, mutationFn: ({ listId, itemId }) => deleteShoppingListItem(listId, itemId), - onSuccess: invalidate, + onMutate: ({ listId, itemId }) => + removeItemOptimistically( + queryClient, + SHOPPING_LIST_QUERY_KEYS.byId(listId), + itemId, + ), + onError: (_error, { listId }, rollback) => + restoreItem(queryClient, SHOPPING_LIST_QUERY_KEYS.byId(listId), rollback), + onSettled: invalidate, }); } @@ -170,25 +215,60 @@ export function useUpdateSharedShoppingList() { } export function useUpdateSharedShoppingListItem() { + const queryClient = useQueryClient(); const invalidate = useInvalidateSharedList(); + return useMutation< ShoppingListItemDto, Error, - { token: string; itemId: string; data: ShoppingListItemRequest } + { token: string; itemId: string; data: ShoppingListItemRequest }, + IItemRollback | undefined >({ mutationKey: OFFLINE_MUTATION_KEYS.sharedItemUpdate, mutationFn: ({ token, itemId, data }) => updateSharedShoppingListItem(token, itemId, data), - onSuccess: (_data, { token }) => invalidate(token), + onMutate: ({ token, itemId, data }) => + patchItemOptimistically( + queryClient, + SHOPPING_LIST_QUERY_KEYS.byToken(token), + itemId, + data, + ), + onError: (_error, { token }, rollback) => + restoreItem( + queryClient, + SHOPPING_LIST_QUERY_KEYS.byToken(token), + rollback, + ), + onSettled: (_data, _error, { token }) => invalidate(token), }); } export function useDeleteSharedShoppingListItem() { + const queryClient = useQueryClient(); const invalidate = useInvalidateSharedList(); - return useMutation({ + + return useMutation< + void, + Error, + { token: string; itemId: string }, + IItemRollback | undefined + >({ mutationKey: OFFLINE_MUTATION_KEYS.sharedItemDelete, mutationFn: ({ token, itemId }) => deleteSharedShoppingListItem(token, itemId), - onSuccess: (_data, { token }) => invalidate(token), + onMutate: ({ token, itemId }) => + removeItemOptimistically( + queryClient, + SHOPPING_LIST_QUERY_KEYS.byToken(token), + itemId, + ), + onError: (_error, { token }, rollback) => + restoreItem( + queryClient, + SHOPPING_LIST_QUERY_KEYS.byToken(token), + rollback, + ), + onSettled: (_data, _error, { token }) => invalidate(token), }); } diff --git a/frontend/src/lib/api/shopping-lists/optimistic-items.ts b/frontend/src/lib/api/shopping-lists/optimistic-items.ts new file mode 100644 index 00000000..05e49a85 --- /dev/null +++ b/frontend/src/lib/api/shopping-lists/optimistic-items.ts @@ -0,0 +1,95 @@ +import type { QueryClient, QueryKey } from "@tanstack/react-query"; + +import type { ShoppingListDto } from "@/lib/api/schemas/shopping-list"; +import type { ShoppingListItemDto } from "@/lib/api/schemas/shopping-list-item"; + +/** + * Optimistic item edits, scoped to the one item being written. + * + * Rolling back a whole-list snapshot looks equivalent and is not: two quick writes + * overlap, so restoring the list as it was before write A also undoes write B, which the + * server has already accepted. The item then reads as unbought while the server has it + * bought, and nothing refetches to correct it. + */ +export interface IItemRollback { + item: ShoppingListItemDto; + index: number; +} + +function findItem( + list: ShoppingListDto | undefined, + itemId: string, +): IItemRollback | undefined { + const index = list?.items?.findIndex((item) => item.id === itemId) ?? -1; + if (!list || index < 0) return undefined; + + return { item: list.items[index], index }; +} + +export async function patchItemOptimistically( + queryClient: QueryClient, + queryKey: QueryKey, + itemId: string, + patch: Partial, +): Promise { + await queryClient.cancelQueries({ queryKey }); + + const previous = findItem( + queryClient.getQueryData(queryKey), + itemId, + ); + if (!previous) return undefined; + + queryClient.setQueryData(queryKey, (old) => + old + ? { + ...old, + items: old.items.map((item) => + item.id === itemId ? { ...item, ...patch } : item, + ), + } + : old, + ); + + return previous; +} + +export async function removeItemOptimistically( + queryClient: QueryClient, + queryKey: QueryKey, + itemId: string, +): Promise { + await queryClient.cancelQueries({ queryKey }); + + const previous = findItem( + queryClient.getQueryData(queryKey), + itemId, + ); + if (!previous) return undefined; + + queryClient.setQueryData(queryKey, (old) => + old + ? { ...old, items: old.items.filter((item) => item.id !== itemId) } + : old, + ); + + return previous; +} + +/** Puts one item back where it was, leaving every other item as it now stands. */ +export function restoreItem( + queryClient: QueryClient, + queryKey: QueryKey, + rollback: IItemRollback | undefined, +) { + if (!rollback) return; + + queryClient.setQueryData(queryKey, (old) => { + if (!old) return old; + + const without = old.items.filter((item) => item.id !== rollback.item.id); + without.splice(rollback.index, 0, rollback.item); + + return { ...old, items: without }; + }); +} diff --git a/frontend/src/lib/offline/offline-mutations.ts b/frontend/src/lib/offline/offline-mutations.ts index edf64a7b..1c2f9e43 100644 --- a/frontend/src/lib/offline/offline-mutations.ts +++ b/frontend/src/lib/offline/offline-mutations.ts @@ -12,6 +12,7 @@ import { deleteSharedShoppingListItem, } from "@/lib/api/shopping-lists"; import { SHOPPING_LIST_QUERY_KEYS } from "@/lib/api/shopping-lists/keys"; +import { parseProblem } from "@/lib/api/problem-details"; import { addToWatchlist, removeFromWatchlist } from "@/lib/api/watchlist"; import type { ShoppingListRequest, @@ -20,13 +21,11 @@ import type { } from "@/lib/api/types"; import { OFFLINE_MUTATION_KEYS } from "@/lib/offline/offline-mutation-keys"; -const SHOPPING_LISTS_ME: QueryKey = ["shoppingLists", "me"]; - function listAndItemsKeys(listId: string): QueryKey[] { return [ - ["shoppingLists", listId], - SHOPPING_LISTS_ME, - ["shoppingListItems", "me"], + SHOPPING_LIST_QUERY_KEYS.byId(listId), + SHOPPING_LIST_QUERY_KEYS.me, + SHOPPING_LIST_QUERY_KEYS.myItems, ]; } @@ -39,7 +38,7 @@ export function registerOfflineMutationDefaults(queryClient: QueryClient) { // A reload also loses the onError passed at mutate() time, so a write that fails on // replay reverts with no explanation. Only pass this for mutations whose call sites // do NOT handle errors themselves, or the user gets the same toast twice. - onError?: () => void, + onError?: (error: Error) => void, ) { queryClient.setMutationDefaults(mutationKey, { mutationFn, @@ -55,20 +54,23 @@ export function registerOfflineMutationDefaults(queryClient: QueryClient) { defineOfflineMutation( OFFLINE_MUTATION_KEYS.shoppingListCreate, (data: ShoppingListRequest) => createShoppingList(data), - () => [SHOPPING_LISTS_ME], + () => [SHOPPING_LIST_QUERY_KEYS.me], ); defineOfflineMutation( OFFLINE_MUTATION_KEYS.shoppingListUpdate, ({ id, data }: { id: string; data: ShoppingListRequest }) => updateShoppingList(id, data), - ({ id }) => [["shoppingLists", id], SHOPPING_LISTS_ME], + ({ id }) => [ + SHOPPING_LIST_QUERY_KEYS.byId(id), + SHOPPING_LIST_QUERY_KEYS.me, + ], ); defineOfflineMutation( OFFLINE_MUTATION_KEYS.shoppingListDelete, (id: string) => deleteShoppingList(id), - (id) => [["shoppingLists", id], SHOPPING_LISTS_ME], + (id) => [SHOPPING_LIST_QUERY_KEYS.byId(id), SHOPPING_LIST_QUERY_KEYS.me], ); defineOfflineMutation( @@ -138,9 +140,19 @@ export function registerOfflineMutationDefaults(queryClient: QueryClient) { /** * Access to a shared list can be withdrawn between queuing a write and replaying it, and * the owner is under no obligation to warn anyone. Saying so beats a silent revert. + * + * Only for 403 and 404 though. This default also runs for live online failures, so + * blaming access loss for every error told a collaborator with perfectly good access + * that they had lost it because a request happened to time out. A 404 additionally + * covers a replayed delete for an item that is already gone, which is harmless. */ -function sharedWriteFailed() { +function sharedWriteFailed(error: Error) { + const status = parseProblem(error)?.status; + const lostAccess = status === 403 || status === 404; + toast.error( - "Promjena nije spremljena. Možda više nemaš pristup ovom popisu.", + lostAccess + ? "Promjena nije spremljena. Možda više nemaš pristup ovom popisu." + : "Promjena nije spremljena. Pokušaj ponovno.", ); } From 1cbdd65e31f1cf8a34c29fff143fea859ed0acaa Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:42:42 +0200 Subject: [PATCH 14/21] fix(shopping-lists): Spell query keys once and drop a revoked list from cache Changes: - Replace every raw ["shoppingLists"] literal with SHOPPING_LIST_QUERY_KEYS - Invalidate the flat item list after a copy, not just the list roots - Remove cached shared lists when sharing is set back to NONE - Give the DTO schema its own object instead of extending the request schema - Encode the token and item id in the four shared request paths Copying a list creates items, but only the list roots were invalidated, so the copied items were invisible to watchlist suggestions for the rest of the session. The DTO schema inherited the create form's title length rules, which the backend does not enforce, so a title the server accepts would have failed to parse the moment response validation was added. It also had ownerId as required, which is no longer true now that the server nulls it for non-owners. Revoking a link killed the token server-side but left the list readable from this browser's cache for the remaining staleTime. --- .../[id]/hooks/use-shopping-list-mutations.ts | 23 ++++++++++++------- .../components/forms/shopping-list-modal.tsx | 3 ++- .../shopping-list-actions-sheet.tsx | 3 ++- .../hooks/use-shopping-list-modal.ts | 5 +++- frontend/src/lib/api/schemas/shopping-list.ts | 14 +++++++---- frontend/src/lib/api/shopping-lists/keys.ts | 1 + .../src/lib/api/shopping-lists/queries.ts | 17 ++++++++++---- 7 files changed, 47 insertions(+), 19 deletions(-) diff --git a/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-mutations.ts b/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-mutations.ts index 51097d1c..616af5de 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-mutations.ts +++ b/frontend/src/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-mutations.ts @@ -8,6 +8,7 @@ import type { ShoppingListRequest, ShoppingListItemRequest, } from "@/lib/api/types"; +import { SHOPPING_LIST_QUERY_KEYS } from "@/lib/api/shopping-lists/keys"; export function useShoppingListMutations( listId: string, @@ -22,13 +23,13 @@ export function useShoppingListMutations( const confirmDelete = async () => { // Prepare optimistic update: remove item from cache immediately - await queryClient.cancelQueries({ queryKey: ["shoppingLists", "me"] }); + await queryClient.cancelQueries({ queryKey: SHOPPING_LIST_QUERY_KEYS.me }); const previous = queryClient.getQueryData([ "shoppingLists", "me", ]); queryClient.setQueryData( - ["shoppingLists", "me"], + SHOPPING_LIST_QUERY_KEYS.me, (old: ShoppingList[] | undefined) => old ? old.filter((l) => l.id !== listId) : [], ); @@ -45,14 +46,14 @@ export function useShoppingListMutations( } catch (error) { // Rollback cache so UI reflects server state if (previous) { - queryClient.setQueryData(["shoppingLists", "me"], previous); + queryClient.setQueryData(SHOPPING_LIST_QUERY_KEYS.me, previous); } toast.error( (error instanceof Error && error.message) || "Greška pri brisanju popisa za kupnju. Pokušaj ponovno.", ); } finally { - queryClient.invalidateQueries({ queryKey: ["shoppingLists", "me"] }); + queryClient.invalidateQueries({ queryKey: SHOPPING_LIST_QUERY_KEYS.me }); } }; @@ -96,10 +97,16 @@ export function useShoppingListMutations( await Promise.all(copyPromises); } - // Invalidate queries to refresh data - await queryClient.invalidateQueries({ - queryKey: ["shoppingLists"], - }); + // Both roots: the copy creates items, and the flat item list feeds watchlist + // suggestions, which would otherwise not see them until something else refetched. + await Promise.all([ + queryClient.invalidateQueries({ + queryKey: SHOPPING_LIST_QUERY_KEYS.all, + }), + queryClient.invalidateQueries({ + queryKey: SHOPPING_LIST_QUERY_KEYS.itemsAll, + }), + ]); // Say the copy is private rather than leaving it to be discovered: someone copying // a shared list may well assume the same people can still reach it. diff --git a/frontend/src/app/(user)/shopping-lists/components/forms/shopping-list-modal.tsx b/frontend/src/app/(user)/shopping-lists/components/forms/shopping-list-modal.tsx index 76c4984b..d22340a9 100644 --- a/frontend/src/app/(user)/shopping-lists/components/forms/shopping-list-modal.tsx +++ b/frontend/src/app/(user)/shopping-lists/components/forms/shopping-list-modal.tsx @@ -26,6 +26,7 @@ import { takeModalError } from "@/lib/modal/modal-error-bus"; import { useFormDraft } from "@/hooks/use-form-draft"; import { getFormDraft } from "@/utils/browser/local-storage"; import { useShoppingListModal } from "@/app/(user)/shopping-lists/hooks/use-shopping-list-modal"; +import { SHOPPING_LIST_QUERY_KEYS } from "@/lib/api/shopping-lists/keys"; interface IShoppingListModalProps { open: boolean; @@ -44,7 +45,7 @@ export default function ShoppingListModal({ // Only seeds an instant value while the reactive by-id query settles; by-id wins // once loaded, since edits invalidate ["shoppingLists"] and refetch it. const cachedList = queryClient - .getQueryData(["shoppingLists", "me"]) + .getQueryData(SHOPPING_LIST_QUERY_KEYS.me) ?.find((list) => list.id === id); const byIdQuery = shoppingListService.useGetShoppingListById( isEdit ? (id as string) : "", diff --git a/frontend/src/app/(user)/shopping-lists/components/shopping-list-actions-sheet.tsx b/frontend/src/app/(user)/shopping-lists/components/shopping-list-actions-sheet.tsx index 16088366..9af76cc8 100644 --- a/frontend/src/app/(user)/shopping-lists/components/shopping-list-actions-sheet.tsx +++ b/frontend/src/app/(user)/shopping-lists/components/shopping-list-actions-sheet.tsx @@ -9,6 +9,7 @@ import { closeModalUrl } from "@/lib/modal/modal-navigation"; import { useShoppingListActions } from "@/app/(user)/shopping-lists/[id]/hooks/use-shopping-list-actions"; import ShoppingListQuickActionsList from "@/app/(user)/shopping-lists/components/shopping-list-quick-actions-list"; import ShoppingListSummary from "@/app/(user)/shopping-lists/components/shopping-list-summary"; +import { SHOPPING_LIST_QUERY_KEYS } from "@/lib/api/shopping-lists/keys"; interface IShoppingListActionsSheetProps { open: boolean; @@ -30,7 +31,7 @@ export default function ShoppingListActionsSheet({ const queryClient = useQueryClient(); const cachedList = queryClient - .getQueryData(["shoppingLists", "me"]) + .getQueryData(SHOPPING_LIST_QUERY_KEYS.me) ?.find((list) => list.id === id); const byIdQuery = shoppingListService.useGetShoppingListById(id); const shoppingList = diff --git a/frontend/src/app/(user)/shopping-lists/hooks/use-shopping-list-modal.ts b/frontend/src/app/(user)/shopping-lists/hooks/use-shopping-list-modal.ts index 07cc4320..4a4a53d5 100644 --- a/frontend/src/app/(user)/shopping-lists/hooks/use-shopping-list-modal.ts +++ b/frontend/src/app/(user)/shopping-lists/hooks/use-shopping-list-modal.ts @@ -6,6 +6,7 @@ import type { ShoppingListDto, ShoppingListRequest } from "@/lib/api/types"; import { stashModalError } from "@/lib/modal/modal-error-bus"; import { closeModalUrl, openModalUrl } from "@/lib/modal/modal-navigation"; import { removeFormDraft } from "@/utils/browser/local-storage"; +import { SHOPPING_LIST_QUERY_KEYS } from "@/lib/api/shopping-lists/keys"; interface IUseShoppingListModalProps { shoppingList?: ShoppingListDto | null; @@ -42,7 +43,9 @@ export function useShoppingListModal({ } removeFormDraft(draftKey); - await queryClient.invalidateQueries({ queryKey: ["shoppingLists"] }); + await queryClient.invalidateQueries({ + queryKey: SHOPPING_LIST_QUERY_KEYS.all, + }); } catch (error) { stashModalError(draftKey, error); openModalUrl( diff --git a/frontend/src/lib/api/schemas/shopping-list.ts b/frontend/src/lib/api/schemas/shopping-list.ts index 85491c81..ad9fc66b 100644 --- a/frontend/src/lib/api/schemas/shopping-list.ts +++ b/frontend/src/lib/api/schemas/shopping-list.ts @@ -21,11 +21,17 @@ export const shoppingListRequestSchema = z.object({ linkAccess: linkAccessSchema.optional(), }); -export const shoppingListDtoSchema = shoppingListRequestSchema.extend({ +// Its own object rather than an extension of the request schema. The request's title +// rules are form validation, which the backend does not enforce, so inheriting them here +// would make a response the server considers valid fail to parse. +export const shoppingListDtoSchema = z.object({ id: z.string(), - ownerId: z.string(), - // Both owner-only: the server sends null to anyone who arrived through a link, so - // they cannot reshare the list at a level its owner never granted. + title: z.string(), + // Owner-only, like the two below: an account id is a stable identifier and a share + // link can travel anywhere. + ownerId: z.string().nullable().optional(), + // The server sends null to anyone who arrived through a link, so they cannot reshare + // the list at a level its owner never granted. linkAccess: linkAccessSchema.nullable().optional(), shareToken: z.string().nullable().optional(), // The caller's resolved access, so the client never re-derives the backend rule. diff --git a/frontend/src/lib/api/shopping-lists/keys.ts b/frontend/src/lib/api/shopping-lists/keys.ts index 2c99b9e2..56936939 100644 --- a/frontend/src/lib/api/shopping-lists/keys.ts +++ b/frontend/src/lib/api/shopping-lists/keys.ts @@ -16,5 +16,6 @@ export const SHOPPING_LIST_QUERY_KEYS = { * its own persistence and purge story. See lib/offline/cache-identity.ts, which is what * makes persisting someone else's list acceptable. */ + sharedRoot: ["sharedShoppingList"] as const, byToken: (token: string) => ["sharedShoppingList", token] as const, }; diff --git a/frontend/src/lib/api/shopping-lists/queries.ts b/frontend/src/lib/api/shopping-lists/queries.ts index f029e74d..32070231 100644 --- a/frontend/src/lib/api/shopping-lists/queries.ts +++ b/frontend/src/lib/api/shopping-lists/queries.ts @@ -92,10 +92,17 @@ export async function getAllUserShoppingListItems(): Promise< // list's id is never enough. apiClient omits the Authorization header when there is no // session, which is exactly the anonymous read path. +// Both are server-generated UUIDs today, so nothing can currently break out of a path +// segment. Encoded anyway, to match shareListUrl and to stay correct if the token format +// ever changes. +function sharedPath(token: string, suffix = ""): string { + return `/api/shared/${encodeURIComponent(token)}${suffix}`; +} + export async function getSharedShoppingList( token: string, ): Promise { - const response = await apiClient.get(`/api/shared/${token}`); + const response = await apiClient.get(sharedPath(token)); return response.data; } @@ -104,7 +111,7 @@ export async function updateSharedShoppingList( data: ShoppingListRequest, ): Promise { const response = await apiClient.put( - `/api/shared/${token}`, + sharedPath(token), data, ); return response.data; @@ -116,7 +123,7 @@ export async function updateSharedShoppingListItem( data: ShoppingListItemRequest, ): Promise { const response = await apiClient.put( - `/api/shared/${token}/items/${itemId}`, + sharedPath(token, `/items/${encodeURIComponent(itemId)}`), data, ); return response.data; @@ -126,5 +133,7 @@ export async function deleteSharedShoppingListItem( token: string, itemId: string, ): Promise { - await apiClient.delete(`/api/shared/${token}/items/${itemId}`); + await apiClient.delete( + sharedPath(token, `/items/${encodeURIComponent(itemId)}`), + ); } From f154141fc1e1661305913d436330e3c33fd03215 Mon Sep 17 00:00:00 2001 From: CrazyFreak <44674613+OffCrazyFreak@users.noreply.github.com> Date: Thu, 6 Aug 2026 00:53:01 +0200 Subject: [PATCH 15/21] fix(shopping-lists): Let a recipient forward the link they are looking at Changes: - Thread the route token into useShoppingListActions, through the action buttons - Show the revoked-link screen only for a 404, with a retry state for anything else - Split the shared page into access-banner and unavailable components - Tell a recipient what access they have, and point disabled controls at it - Make isSignedIn required rather than defaulting to true - Add a timeout to the server-side preview fetch - Widen onShare to take modal options, and stop passing it straight to onClick - Give useGetShoppingListById an explicit enabled instead of an empty id - Make pluralizeCroatian three-form and drop the server-only item counter The headline bug: shareToken is nulled for everyone but the owner, and on /s/ the caller is by definition not the owner, so the share action always fell through to plain text while the link sat in the address bar. Forwarding a shared list is the most likely thing to do on that page and it silently did the wrong thing. Every query error rendered as "the owner stopped sharing this list", so a shopper on flaky mobile data was told their access had been revoked and sent off to ask for a link they already had. Only a 404 means that. A recipient was never told what they could do, so the greyed-out checkbox and store select had no stated reason, which a screen reader renders as "dimmed" and nothing more. Notes: - Widening onShare surfaced a latent hazard: it was passed directly as onClick, so a MouseEvent would have arrived where IOpenModalOptions was expected. - Kept the checkbox and store select disabled rather than hidden, which is the opposite of what the plan said. Their state is information a viewer needs (what has been bought, from which shop), so hiding them would remove it. Explaining them costs nothing. - pluralizeCroatian was two-form and could not produce the 5-and-up genitive. Its two existing callers were accidentally correct, because for "trgovina" and "cijena" that form matches the singular. It now takes a third form, defaulting to the first so those callers are unchanged. --- .../components/items/shopping-list-item.tsx | 3 + .../shopping-list-action-buttons.tsx | 5 +- .../shopping-list-desktop-actions.tsx | 2 +- .../shopping-list-detail-client.tsx | 3 +- .../[id]/components/shopping-list-header.tsx | 11 ++- .../shopping-list-mobile-actions.tsx | 2 +- .../[id]/hooks/use-shopping-list-actions.ts | 17 +++-- .../[id]/hooks/use-shopping-list-data.ts | 12 ++-- .../components/shared-list-access-banner.tsx | 72 +++++++++++++++++++ .../components/shared-list-unavailable.tsx | 64 +++++++++++++++++ .../shared-shopping-list-client.tsx | 48 +++---------- .../app/s/[token]/get-shared-list-preview.ts | 22 +++--- frontend/src/app/s/[token]/page.tsx | 9 ++- .../custom/store-chain/store-chain-select.tsx | 8 ++- frontend/src/utils/strings.ts | 33 ++++++--- 15 files changed, 226 insertions(+), 85 deletions(-) create mode 100644 frontend/src/app/s/[token]/components/shared-list-access-banner.tsx create mode 100644 frontend/src/app/s/[token]/components/shared-list-unavailable.tsx diff --git a/frontend/src/app/(user)/shopping-lists/[id]/components/items/shopping-list-item.tsx b/frontend/src/app/(user)/shopping-lists/[id]/components/items/shopping-list-item.tsx index 6661e1ef..300a4690 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/components/items/shopping-list-item.tsx +++ b/frontend/src/app/(user)/shopping-lists/[id]/components/items/shopping-list-item.tsx @@ -11,6 +11,7 @@ import ItemPriceDisplay from "@/app/(user)/shopping-lists/[id]/components/items/ import type { IShoppingListItemUpdate } from "@/app/(user)/shopping-lists/[id]/typings/shopping-list-item-types"; import { cn } from "@/lib/utils"; import { productPath } from "@/utils/product-links"; +import { SHARED_ACCESS_BANNER_ID } from "@/app/s/[token]/components/shared-list-access-banner"; interface IShoppingListItemProps { item: ShoppingListItemDto; @@ -57,6 +58,7 @@ export default function ShoppingListItem({ className="relative z-20" checked={item.isChecked} disabled={!canCheck} + aria-describedby={canCheck ? undefined : SHARED_ACCESS_BANNER_ID} onCheckedChange={(checked) => onUpdate({ isChecked: checked as boolean, @@ -127,6 +129,7 @@ export default function ShoppingListItem({ }) } disabled={item.isChecked || !canCheck} + describedById={canCheck ? undefined : SHARED_ACCESS_BANNER_ID} defaultValue={cheapestStore} storePrices={storePrices} averagePrice={averagePrice} diff --git a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-action-buttons.tsx b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-action-buttons.tsx index 164fe3bb..2be58645 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-action-buttons.tsx +++ b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-action-buttons.tsx @@ -10,6 +10,8 @@ interface IShoppingListActionButtonsProps { showEditButton?: boolean; showDeleteButton?: boolean; showShareButton?: boolean; + /** Set when the page was reached through a share link, so share can offer that link. */ + shareToken?: string; mobilePresentation?: "menu" | "buttons" | "none"; className?: string; } @@ -20,6 +22,7 @@ export default function ShoppingListActionButtons({ showEditButton = false, showDeleteButton = false, showShareButton = false, + shareToken, mobilePresentation = "menu", className, }: IShoppingListActionButtonsProps) { @@ -32,7 +35,7 @@ export default function ShoppingListActionButtons({ handleEdit, handleShare, handleCopy, - } = useShoppingListActions(shoppingList); + } = useShoppingListActions(shoppingList, shareToken); const groupProps = { showShareButton, diff --git a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-desktop-actions.tsx b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-desktop-actions.tsx index 9ebedb9b..34b49d27 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-desktop-actions.tsx +++ b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-desktop-actions.tsx @@ -51,7 +51,7 @@ export default function ShoppingListDesktopActions({ size="icon" aria-label={shareLabel} className="shrink-0" - onClick={onShare} + onClick={() => onShare()} >

{/* Header Section */}
- + {/* This route is behind the auth gate, so the caller is always signed in. */} + {listUpdatedAt > 0 && (
diff --git a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-mobile-actions.tsx b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-mobile-actions.tsx index 0029e24d..b428ad8e 100644 --- a/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-mobile-actions.tsx +++ b/frontend/src/app/(user)/shopping-lists/[id]/components/shopping-list-mobile-actions.tsx @@ -52,7 +52,7 @@ export default function ShoppingListMobileActions({ {showShareButton && ( onShare()} className="cursor-pointer flex items-center gap-4" >