mirror of
https://github.com/Stirling-Tools/Stirling-PDF.git
synced 2026-09-03 05:10:16 +03:00
Move usage-ranked tool suggestions to the post-operation panel
This commit is contained in:
-40
@@ -5,7 +5,6 @@ import java.util.Optional;
|
||||
import java.util.regex.Pattern;
|
||||
|
||||
import org.springframework.http.ResponseEntity;
|
||||
import org.springframework.web.bind.annotation.DeleteMapping;
|
||||
import org.springframework.web.bind.annotation.GetMapping;
|
||||
import org.springframework.web.bind.annotation.PostMapping;
|
||||
import org.springframework.web.bind.annotation.RequestBody;
|
||||
@@ -19,7 +18,6 @@ import lombok.extern.slf4j.Slf4j;
|
||||
|
||||
import stirling.software.common.annotations.api.ProprietaryUiDataApi;
|
||||
import stirling.software.common.service.UserServiceInterface;
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissal;
|
||||
import stirling.software.proprietary.service.ToolRecommendationService;
|
||||
import stirling.software.proprietary.service.ToolRecommendationService.ToolRecommendation;
|
||||
import stirling.software.proprietary.service.ToolRecommendationService.ToolWorkflow;
|
||||
@@ -50,8 +48,6 @@ public class ToolRecommendationController {
|
||||
*/
|
||||
public record UsageRequest(String toolKey, List<List<String>> priorChains) {}
|
||||
|
||||
public record DismissalRequest(String contextTool, String dismissedTool) {}
|
||||
|
||||
@GetMapping("/tool-recommendations")
|
||||
@Operation(
|
||||
summary = "Get recommended tools",
|
||||
@@ -117,42 +113,6 @@ public class ToolRecommendationController {
|
||||
return ResponseEntity.noContent().build();
|
||||
}
|
||||
|
||||
@PostMapping("/tool-recommendations/dismissals")
|
||||
@Operation(
|
||||
summary = "Dismiss a recommended tool",
|
||||
description =
|
||||
"Never recommend dismissedTool again while on contextTool. Use context '*' to"
|
||||
+ " suppress it everywhere.")
|
||||
public ResponseEntity<Void> dismiss(
|
||||
@RequestBody DismissalRequest request,
|
||||
@RequestHeader(value = "X-Browser-Id", required = false) String browserId) {
|
||||
if (request == null || !isValidDismissal(request.contextTool(), request.dismissedTool())) {
|
||||
return ResponseEntity.badRequest().build();
|
||||
}
|
||||
recommendationService.dismiss(
|
||||
resolvePrincipal(browserId), request.contextTool(), request.dismissedTool());
|
||||
return ResponseEntity.noContent().build();
|
||||
}
|
||||
|
||||
@DeleteMapping("/tool-recommendations/dismissals")
|
||||
@Operation(summary = "Undo a recommendation dismissal")
|
||||
public ResponseEntity<Void> undoDismiss(
|
||||
@RequestParam("contextTool") String contextTool,
|
||||
@RequestParam("dismissedTool") String dismissedTool,
|
||||
@RequestHeader(value = "X-Browser-Id", required = false) String browserId) {
|
||||
if (!isValidDismissal(contextTool, dismissedTool)) {
|
||||
return ResponseEntity.badRequest().build();
|
||||
}
|
||||
recommendationService.undoDismiss(resolvePrincipal(browserId), contextTool, dismissedTool);
|
||||
return ResponseEntity.noContent().build();
|
||||
}
|
||||
|
||||
private static boolean isValidDismissal(String contextTool, String dismissedTool) {
|
||||
return ToolUsageTrackingService.isValidToolKey(dismissedTool)
|
||||
&& (ToolRecommendationDismissal.ANY_CONTEXT.equals(contextTool)
|
||||
|| ToolUsageTrackingService.isValidToolKey(contextTool));
|
||||
}
|
||||
|
||||
/** Logged-in username, else a per-browser pseudo-identity, else a shared anonymous bucket. */
|
||||
private String resolvePrincipal(String browserId) {
|
||||
String username =
|
||||
|
||||
-52
@@ -1,52 +0,0 @@
|
||||
package stirling.software.proprietary.model;
|
||||
|
||||
import java.io.Serializable;
|
||||
|
||||
import jakarta.persistence.Column;
|
||||
import jakarta.persistence.Entity;
|
||||
import jakarta.persistence.Id;
|
||||
import jakarta.persistence.IdClass;
|
||||
import jakarta.persistence.Index;
|
||||
import jakarta.persistence.Table;
|
||||
|
||||
import lombok.Getter;
|
||||
import lombok.NoArgsConstructor;
|
||||
import lombok.Setter;
|
||||
|
||||
/**
|
||||
* A principal's opt-out: never recommend {@code dismissedTool} while they are on {@code
|
||||
* contextTool}. A context of {@code *} suppresses the tool in every context. The whole row is its
|
||||
* own key, so saving one twice is a no-op rather than a duplicate.
|
||||
*/
|
||||
@Entity
|
||||
@Table(
|
||||
name = "tool_recommendation_dismissals",
|
||||
indexes = @Index(name = "idx_tool_rec_dismissal_principal", columnList = "principal"))
|
||||
@IdClass(ToolRecommendationDismissalId.class)
|
||||
@Getter
|
||||
@Setter
|
||||
@NoArgsConstructor
|
||||
public class ToolRecommendationDismissal implements Serializable {
|
||||
|
||||
private static final long serialVersionUID = 1L;
|
||||
|
||||
public static final String ANY_CONTEXT = "*";
|
||||
|
||||
@Id
|
||||
@Column(name = "principal", length = 255)
|
||||
private String principal;
|
||||
|
||||
@Id
|
||||
@Column(name = "context_tool", length = 64)
|
||||
private String contextTool;
|
||||
|
||||
@Id
|
||||
@Column(name = "dismissed_tool", length = 64)
|
||||
private String dismissedTool;
|
||||
|
||||
public ToolRecommendationDismissal(String principal, String contextTool, String dismissedTool) {
|
||||
this.principal = principal;
|
||||
this.contextTool = contextTool;
|
||||
this.dismissedTool = dismissedTool;
|
||||
}
|
||||
}
|
||||
-19
@@ -1,19 +0,0 @@
|
||||
package stirling.software.proprietary.model;
|
||||
|
||||
import java.io.Serializable;
|
||||
|
||||
import lombok.AllArgsConstructor;
|
||||
import lombok.Data;
|
||||
import lombok.NoArgsConstructor;
|
||||
|
||||
@Data
|
||||
@NoArgsConstructor
|
||||
@AllArgsConstructor
|
||||
public class ToolRecommendationDismissalId implements Serializable {
|
||||
|
||||
private static final long serialVersionUID = 1L;
|
||||
|
||||
private String principal;
|
||||
private String contextTool;
|
||||
private String dismissedTool;
|
||||
}
|
||||
-27
@@ -1,27 +0,0 @@
|
||||
package stirling.software.proprietary.repository;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
import org.springframework.data.jpa.repository.JpaRepository;
|
||||
import org.springframework.data.jpa.repository.Modifying;
|
||||
import org.springframework.data.jpa.repository.Query;
|
||||
import org.springframework.data.repository.query.Param;
|
||||
import org.springframework.stereotype.Repository;
|
||||
import org.springframework.transaction.annotation.Transactional;
|
||||
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissal;
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissalId;
|
||||
|
||||
@Repository
|
||||
public interface ToolRecommendationDismissalRepository
|
||||
extends JpaRepository<ToolRecommendationDismissal, ToolRecommendationDismissalId> {
|
||||
|
||||
List<ToolRecommendationDismissal> findByPrincipal(String principal);
|
||||
|
||||
// Erasure: rows key on the raw username, so a recreated name would inherit the opt-outs.
|
||||
// No clearAutomatically: a clear would detach the User deleteUser deletes right after this.
|
||||
@Modifying
|
||||
@Transactional
|
||||
@Query("DELETE FROM ToolRecommendationDismissal d WHERE d.principal = :principal")
|
||||
int deleteByPrincipal(@Param("principal") String principal);
|
||||
}
|
||||
+1
-4
@@ -46,7 +46,6 @@ import stirling.software.proprietary.integration.model.IntegrationConfig;
|
||||
import stirling.software.proprietary.integration.repository.IntegrationConfigRepository;
|
||||
import stirling.software.proprietary.model.Team;
|
||||
import stirling.software.proprietary.repository.ToolChainStatRepository;
|
||||
import stirling.software.proprietary.repository.ToolRecommendationDismissalRepository;
|
||||
import stirling.software.proprietary.repository.ToolUsageStatRepository;
|
||||
import stirling.software.proprietary.security.database.repository.AuthorityRepository;
|
||||
import stirling.software.proprietary.security.database.repository.PersistentLoginRepository;
|
||||
@@ -101,7 +100,6 @@ public class UserService implements UserServiceInterface {
|
||||
private final ApiKeyAuthenticationService apiKeyAuthenticationService;
|
||||
private final ToolUsageStatRepository toolUsageStatRepository;
|
||||
private final ToolChainStatRepository toolChainStatRepository;
|
||||
private final ToolRecommendationDismissalRepository toolRecommendationDismissalRepository;
|
||||
|
||||
@Transactional
|
||||
public void processSSOPostLogin(
|
||||
@@ -269,10 +267,9 @@ public class UserService implements UserServiceInterface {
|
||||
private void deleteUserRelatedData(User user) {
|
||||
log.info("Deleting all associated data for user: {}", user.getUsername());
|
||||
|
||||
// Tool usage and dismissals key on the username, so a recreated name would inherit them
|
||||
// Tool usage keys on the username, so a recreated name would inherit it
|
||||
toolUsageStatRepository.deleteByPrincipal(user.getUsername());
|
||||
toolChainStatRepository.deleteByPrincipal(user.getUsername());
|
||||
toolRecommendationDismissalRepository.deleteByPrincipal(user.getUsername());
|
||||
|
||||
// Drop ACL grants held by this user and detach grants they issued
|
||||
resourceGrantRepository.deleteByPrincipalTypeAndPrincipalId(
|
||||
|
||||
+4
-50
@@ -7,27 +7,20 @@ import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
import org.springframework.dao.DataIntegrityViolationException;
|
||||
import org.springframework.stereotype.Service;
|
||||
import org.springframework.transaction.annotation.Transactional;
|
||||
|
||||
import lombok.RequiredArgsConstructor;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
|
||||
import stirling.software.common.model.ApplicationProperties;
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissal;
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissalId;
|
||||
import stirling.software.proprietary.repository.ToolRecommendationDismissalRepository;
|
||||
import stirling.software.proprietary.service.ToolUsageSignalService.TeamScope;
|
||||
import stirling.software.proprietary.service.ToolUsageSignalService.ToolChainSummary;
|
||||
|
||||
/**
|
||||
* Scores "what tool next". Transitions out of the current tool dominate, then the caller's own
|
||||
* usage, their team's, and the whole install's. Scoring is deliberately uncached - the costly
|
||||
* aggregates are cached inside {@link ToolUsageSignalService} and shared by everyone, so a
|
||||
* dismissal takes effect immediately without invalidating anyone else's data.
|
||||
* usage, their team's, and the whole install's. Scoring itself is cheap and uncached; the costly
|
||||
* aggregates are cached inside {@link ToolUsageSignalService} and shared by everyone.
|
||||
*/
|
||||
@Slf4j
|
||||
@Service
|
||||
@RequiredArgsConstructor
|
||||
public class ToolRecommendationService {
|
||||
@@ -47,7 +40,6 @@ public class ToolRecommendationService {
|
||||
static final int MIN_WORKFLOW_TOOLS = 2;
|
||||
|
||||
private final ToolUsageSignalService signals;
|
||||
private final ToolRecommendationDismissalRepository dismissalRepository;
|
||||
private final ApplicationProperties applicationProperties;
|
||||
|
||||
public record ToolRecommendation(String toolKey, double score) {}
|
||||
@@ -162,12 +154,9 @@ public class ToolRecommendationService {
|
||||
}
|
||||
merge(scores, signals.globalFrequency(cutoff, recent), WEIGHT_FREQUENCY_GLOBAL);
|
||||
|
||||
Set<String> excluded = dismissedTools(principal, currentTool);
|
||||
if (currentTool != null) {
|
||||
excluded.add(currentTool);
|
||||
}
|
||||
// Never answer "what next" with the tool the user is already in.
|
||||
return scores.entrySet().stream()
|
||||
.filter(e -> !excluded.contains(e.getKey()))
|
||||
.filter(e -> !e.getKey().equals(currentTool))
|
||||
.sorted(
|
||||
Map.Entry.<String, Double>comparingByValue()
|
||||
.reversed()
|
||||
@@ -189,41 +178,6 @@ public class ToolRecommendationService {
|
||||
signal.forEach((tool, value) -> scores.merge(tool, weight * (value / max), Double::sum));
|
||||
}
|
||||
|
||||
private Set<String> dismissedTools(String principal, String currentTool) {
|
||||
Set<String> excluded = new HashSet<>();
|
||||
for (ToolRecommendationDismissal dismissal :
|
||||
dismissalRepository.findByPrincipal(principal)) {
|
||||
String context = dismissal.getContextTool();
|
||||
if (ToolRecommendationDismissal.ANY_CONTEXT.equals(context)
|
||||
|| context.equals(currentTool)) {
|
||||
excluded.add(dismissal.getDismissedTool());
|
||||
}
|
||||
}
|
||||
return excluded;
|
||||
}
|
||||
|
||||
/**
|
||||
* Idempotent: the row is its own primary key. Two simultaneous dismissals (double click, or two
|
||||
* nodes) can still race to insert it, and losing that race already means the desired row
|
||||
* exists.
|
||||
*/
|
||||
@Transactional
|
||||
public void dismiss(String principal, String contextTool, String dismissedTool) {
|
||||
try {
|
||||
dismissalRepository.save(
|
||||
new ToolRecommendationDismissal(principal, contextTool, dismissedTool));
|
||||
} catch (DataIntegrityViolationException e) {
|
||||
log.debug("Dismissal {}/{} already stored", contextTool, dismissedTool);
|
||||
}
|
||||
}
|
||||
|
||||
@Transactional
|
||||
public void undoDismiss(String principal, String contextTool, String dismissedTool) {
|
||||
dismissalRepository
|
||||
.findById(new ToolRecommendationDismissalId(principal, contextTool, dismissedTool))
|
||||
.ifPresent(dismissalRepository::delete);
|
||||
}
|
||||
|
||||
private static double round(double value) {
|
||||
return Math.round(value * 1000.0) / 1000.0;
|
||||
}
|
||||
|
||||
-60
@@ -22,7 +22,6 @@ import org.mockito.junit.jupiter.MockitoExtension;
|
||||
import org.springframework.http.ResponseEntity;
|
||||
|
||||
import stirling.software.common.service.UserServiceInterface;
|
||||
import stirling.software.proprietary.controller.api.ToolRecommendationController.DismissalRequest;
|
||||
import stirling.software.proprietary.controller.api.ToolRecommendationController.RecommendationsResponse;
|
||||
import stirling.software.proprietary.controller.api.ToolRecommendationController.UsageRequest;
|
||||
import stirling.software.proprietary.controller.api.ToolRecommendationController.WorkflowsResponse;
|
||||
@@ -205,63 +204,4 @@ class ToolRecommendationControllerTest {
|
||||
verify(trackingService).recordUsage("anon:" + BROWSER_ID, "ocr", null);
|
||||
}
|
||||
}
|
||||
|
||||
@Nested
|
||||
@DisplayName("Dismissals")
|
||||
class Dismissals {
|
||||
|
||||
@Test
|
||||
@DisplayName("a context-scoped dismissal is stored")
|
||||
void dismissStored() {
|
||||
when(userService.getCurrentUsername()).thenReturn("alice");
|
||||
|
||||
ResponseEntity<Void> response =
|
||||
controller.dismiss(new DismissalRequest("compare", "ocr"), null);
|
||||
|
||||
assertThat(response.getStatusCode().value()).isEqualTo(204);
|
||||
verify(recommendationService).dismiss("alice", "compare", "ocr");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("the any-context wildcard is accepted")
|
||||
void wildcardContextAccepted() {
|
||||
when(userService.getCurrentUsername()).thenReturn("alice");
|
||||
|
||||
ResponseEntity<Void> response =
|
||||
controller.dismiss(new DismissalRequest("*", "ocr"), null);
|
||||
|
||||
assertThat(response.getStatusCode().value()).isEqualTo(204);
|
||||
verify(recommendationService).dismiss("alice", "*", "ocr");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("junk context or tool is rejected with 400")
|
||||
void junkRejected() {
|
||||
assertThat(
|
||||
controller
|
||||
.dismiss(new DismissalRequest("bad context!", "ocr"), null)
|
||||
.getStatusCode()
|
||||
.value())
|
||||
.isEqualTo(400);
|
||||
assertThat(
|
||||
controller
|
||||
.dismiss(new DismissalRequest("compare", "bad tool!"), null)
|
||||
.getStatusCode()
|
||||
.value())
|
||||
.isEqualTo(400);
|
||||
assertThat(controller.dismiss(null, null).getStatusCode().value()).isEqualTo(400);
|
||||
verifyNoInteractions(recommendationService);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("undo removes the stored dismissal")
|
||||
void undoRemoves() {
|
||||
when(userService.getCurrentUsername()).thenReturn("alice");
|
||||
|
||||
ResponseEntity<Void> response = controller.undoDismiss("compare", "ocr", null);
|
||||
|
||||
assertThat(response.getStatusCode().value()).isEqualTo(204);
|
||||
verify(recommendationService).undoDismiss("alice", "compare", "ocr");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
-48
@@ -18,8 +18,6 @@ import org.springframework.data.domain.PageRequest;
|
||||
import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
|
||||
|
||||
import stirling.software.proprietary.model.ToolChainStat;
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissal;
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissalId;
|
||||
import stirling.software.proprietary.model.ToolUsageStat;
|
||||
|
||||
/** Exercises the windowed CASE aggregation and increment queries against H2. */
|
||||
@@ -31,7 +29,6 @@ class ToolRecommendationRepositoriesTest {
|
||||
|
||||
@Autowired private ToolUsageStatRepository usageRepository;
|
||||
@Autowired private ToolChainStatRepository chainRepository;
|
||||
@Autowired private ToolRecommendationDismissalRepository dismissalRepository;
|
||||
|
||||
private static Map<String, long[]> byTool(List<Object[]> rows) {
|
||||
return rows.stream()
|
||||
@@ -275,35 +272,6 @@ class ToolRecommendationRepositoriesTest {
|
||||
assertThat(chainRepository.findAll().get(0).getChainKey()).hasSize(key.length());
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("saving the same dismissal twice leaves one row")
|
||||
void dismissalsAreIdempotent() {
|
||||
dismissalRepository.saveAndFlush(
|
||||
new ToolRecommendationDismissal("alice", "compare", "ocr"));
|
||||
dismissalRepository.saveAndFlush(
|
||||
new ToolRecommendationDismissal("alice", "compare", "ocr"));
|
||||
|
||||
assertThat(dismissalRepository.findByPrincipal("alice")).hasSize(1);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("dismissals are addressable by their full composite key")
|
||||
void dismissalsAddressableByKey() {
|
||||
dismissalRepository.saveAndFlush(
|
||||
new ToolRecommendationDismissal("alice", "compare", "ocr"));
|
||||
dismissalRepository.saveAndFlush(new ToolRecommendationDismissal("alice", "merge", "ocr"));
|
||||
|
||||
assertThat(
|
||||
dismissalRepository.findById(
|
||||
new ToolRecommendationDismissalId("alice", "compare", "ocr")))
|
||||
.isPresent();
|
||||
assertThat(
|
||||
dismissalRepository.findById(
|
||||
new ToolRecommendationDismissalId("alice", "split", "ocr")))
|
||||
.isEmpty();
|
||||
assertThat(dismissalRepository.findByPrincipal("alice")).hasSize(2);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("erasure removes a principal's usage rows and leaves everyone else's")
|
||||
void deleteByPrincipalErasesUsage() {
|
||||
@@ -319,22 +287,6 @@ class ToolRecommendationRepositoriesTest {
|
||||
assertThat(usageRepository.sumByPrincipal("alice", DAY - 30, DAY - 7)).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("erasure removes a principal's dismissals and leaves everyone else's")
|
||||
void deleteByPrincipalErasesDismissals() {
|
||||
dismissalRepository.saveAndFlush(
|
||||
new ToolRecommendationDismissal("alice", "compare", "ocr"));
|
||||
dismissalRepository.saveAndFlush(
|
||||
new ToolRecommendationDismissal(
|
||||
"alice", ToolRecommendationDismissal.ANY_CONTEXT, "merge"));
|
||||
dismissalRepository.saveAndFlush(new ToolRecommendationDismissal("bob", "compare", "ocr"));
|
||||
|
||||
assertThat(dismissalRepository.deleteByPrincipal("alice")).isEqualTo(2);
|
||||
|
||||
assertThat(dismissalRepository.findByPrincipal("alice")).isEmpty();
|
||||
assertThat(dismissalRepository.findByPrincipal("bob")).hasSize(1);
|
||||
}
|
||||
|
||||
@SpringBootConfiguration
|
||||
@EntityScan(
|
||||
basePackages = {
|
||||
|
||||
-7
@@ -15,7 +15,6 @@ import org.springframework.data.jpa.repository.config.EnableJpaRepositories;
|
||||
import jakarta.persistence.EntityManager;
|
||||
|
||||
import stirling.software.proprietary.model.ToolChainStat;
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissal;
|
||||
import stirling.software.proprietary.model.ToolUsageStat;
|
||||
import stirling.software.proprietary.security.database.repository.UserRepository;
|
||||
import stirling.software.proprietary.security.model.Authority;
|
||||
@@ -35,7 +34,6 @@ class ToolUsageErasureDeletesUserTest {
|
||||
@Autowired private UserRepository userRepository;
|
||||
@Autowired private ToolUsageStatRepository usageRepository;
|
||||
@Autowired private ToolChainStatRepository chainRepository;
|
||||
@Autowired private ToolRecommendationDismissalRepository dismissalRepository;
|
||||
@Autowired private EntityManager entityManager;
|
||||
|
||||
private Long seedUser(String username) {
|
||||
@@ -49,7 +47,6 @@ class ToolUsageErasureDeletesUserTest {
|
||||
usageRepository.save(new ToolUsageStat(username, NONE, "ocr", DAY, 1));
|
||||
usageRepository.save(new ToolUsageStat(username, "compare", "merge", DAY - 5, 2));
|
||||
chainRepository.save(new ToolChainStat(username, "compare>merge", DAY, 2, 2));
|
||||
dismissalRepository.save(new ToolRecommendationDismissal(username, "compare", "ocr"));
|
||||
entityManager.flush();
|
||||
entityManager.clear();
|
||||
return user.getId();
|
||||
@@ -75,7 +72,6 @@ class ToolUsageErasureDeletesUserTest {
|
||||
|
||||
usageRepository.deleteByPrincipal("tracked");
|
||||
chainRepository.deleteByPrincipal("tracked");
|
||||
dismissalRepository.deleteByPrincipal("tracked");
|
||||
|
||||
// The erasures must leave the user managed, or delete() merges (and cascades) instead
|
||||
assertThat(entityManager.contains(user)).isTrue();
|
||||
@@ -100,11 +96,9 @@ class ToolUsageErasureDeletesUserTest {
|
||||
assertThat(chainRepository.findAll())
|
||||
.extracting(ToolChainStat::getPrincipal)
|
||||
.containsOnly("bystander");
|
||||
assertThat(dismissalRepository.findByPrincipal("tracked")).isEmpty();
|
||||
|
||||
// The bystander is untouched by any of it
|
||||
assertThat(userRepository.findByUsernameIgnoreCase("bystander")).isPresent();
|
||||
assertThat(dismissalRepository.findByPrincipal("bystander")).hasSize(1);
|
||||
assertThat(countSettings(keptId)).isEqualTo(1);
|
||||
}
|
||||
|
||||
@@ -118,7 +112,6 @@ class ToolUsageErasureDeletesUserTest {
|
||||
|
||||
usageRepository.deleteByPrincipal("tracked");
|
||||
chainRepository.deleteByPrincipal("tracked");
|
||||
dismissalRepository.deleteByPrincipal("tracked");
|
||||
|
||||
assertThat(entityManager.contains(user)).isTrue();
|
||||
assertThat(before).isNotEmpty();
|
||||
|
||||
+1
-4
@@ -27,7 +27,6 @@ import stirling.software.common.model.exception.UnsupportedProviderException;
|
||||
import stirling.software.proprietary.access.repository.ResourceGrantRepository;
|
||||
import stirling.software.proprietary.model.Team;
|
||||
import stirling.software.proprietary.repository.ToolChainStatRepository;
|
||||
import stirling.software.proprietary.repository.ToolRecommendationDismissalRepository;
|
||||
import stirling.software.proprietary.repository.ToolUsageStatRepository;
|
||||
import stirling.software.proprietary.security.database.repository.AuthorityRepository;
|
||||
import stirling.software.proprietary.security.database.repository.PersistentLoginRepository;
|
||||
@@ -77,7 +76,6 @@ class UserServiceTest {
|
||||
@Mock private ApiKeyAuthenticationService apiKeyAuthenticationService;
|
||||
@Mock private ToolUsageStatRepository toolUsageStatRepository;
|
||||
@Mock private ToolChainStatRepository toolChainStatRepository;
|
||||
@Mock private ToolRecommendationDismissalRepository toolRecommendationDismissalRepository;
|
||||
|
||||
@Spy @InjectMocks private UserService userService;
|
||||
|
||||
@@ -292,7 +290,7 @@ class UserServiceTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void deleteUser_erasesToolUsageAndDismissals() {
|
||||
void deleteUser_erasesToolUsage() {
|
||||
User user = new User();
|
||||
user.setId(4L);
|
||||
user.setUsername("tracked");
|
||||
@@ -307,7 +305,6 @@ class UserServiceTest {
|
||||
// Every table keys on the username, so a recreated name would inherit the old profile
|
||||
verify(toolUsageStatRepository).deleteByPrincipal("tracked");
|
||||
verify(toolChainStatRepository).deleteByPrincipal("tracked");
|
||||
verify(toolRecommendationDismissalRepository).deleteByPrincipal("tracked");
|
||||
// The erasures must not displace the user row itself
|
||||
verify(userRepository).delete(user);
|
||||
}
|
||||
|
||||
+3
-115
@@ -14,21 +14,16 @@ import static org.mockito.Mockito.when;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Optional;
|
||||
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.DisplayName;
|
||||
import org.junit.jupiter.api.Nested;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
import org.mockito.ArgumentCaptor;
|
||||
import org.mockito.Mock;
|
||||
import org.mockito.junit.jupiter.MockitoExtension;
|
||||
|
||||
import stirling.software.common.model.ApplicationProperties;
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissal;
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissalId;
|
||||
import stirling.software.proprietary.repository.ToolRecommendationDismissalRepository;
|
||||
import stirling.software.proprietary.service.ToolRecommendationService.ToolRecommendation;
|
||||
import stirling.software.proprietary.service.ToolRecommendationService.ToolWorkflow;
|
||||
import stirling.software.proprietary.service.ToolRecommendationService.WorkflowScope;
|
||||
@@ -42,7 +37,6 @@ class ToolRecommendationServiceTest {
|
||||
private static final TeamScope TEAM = new TeamScope(7L, List.of("bob", "carol"));
|
||||
|
||||
@Mock private ToolUsageSignalService signalService;
|
||||
@Mock private ToolRecommendationDismissalRepository dismissalRepository;
|
||||
|
||||
private ApplicationProperties properties;
|
||||
private ToolRecommendationService service;
|
||||
@@ -51,8 +45,7 @@ class ToolRecommendationServiceTest {
|
||||
void setUp() {
|
||||
properties = new ApplicationProperties();
|
||||
properties.getSystem().setEnableAnalytics(true);
|
||||
service = new ToolRecommendationService(signalService, dismissalRepository, properties);
|
||||
lenient().when(dismissalRepository.findByPrincipal(anyString())).thenReturn(List.of());
|
||||
service = new ToolRecommendationService(signalService, properties);
|
||||
lenient().when(signalService.resolveTeamScope(anyString())).thenReturn(TeamScope.none());
|
||||
lenient()
|
||||
.when(signalService.userFrequency(anyString(), anyLong(), anyLong()))
|
||||
@@ -304,7 +297,7 @@ class ToolRecommendationServiceTest {
|
||||
properties.getToolRecommendations().setEnabled(false);
|
||||
|
||||
assertThat(service.getRecommendations(PRINCIPAL, "compare", 6)).isEmpty();
|
||||
verifyNoInteractions(signalService, dismissalRepository);
|
||||
verifyNoInteractions(signalService);
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -313,7 +306,7 @@ class ToolRecommendationServiceTest {
|
||||
properties.getSystem().setEnableAnalytics(null);
|
||||
|
||||
assertThat(service.getRecommendations(PRINCIPAL, "compare", 6)).isEmpty();
|
||||
verifyNoInteractions(signalService, dismissalRepository);
|
||||
verifyNoInteractions(signalService);
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -326,109 +319,4 @@ class ToolRecommendationServiceTest {
|
||||
verify(signalService, never()).globalTransitions(anyString(), anyLong(), anyLong());
|
||||
}
|
||||
}
|
||||
|
||||
@Nested
|
||||
@DisplayName("Dismissals")
|
||||
class Dismissals {
|
||||
|
||||
@Test
|
||||
@DisplayName("a dismissal for the current context hides the tool")
|
||||
void contextDismissalFilters() {
|
||||
when(signalService.userFrequency(eq(PRINCIPAL), anyLong(), anyLong()))
|
||||
.thenReturn(Map.of("ocr", 6.0, "merge", 5.0));
|
||||
when(dismissalRepository.findByPrincipal(PRINCIPAL))
|
||||
.thenReturn(
|
||||
List.of(new ToolRecommendationDismissal(PRINCIPAL, "compare", "ocr")));
|
||||
|
||||
List<ToolRecommendation> result = service.getRecommendations(PRINCIPAL, "compare", 6);
|
||||
|
||||
assertThat(toolKeys(result)).containsExactly("merge");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("a dismissal for another context does not hide the tool")
|
||||
void unrelatedContextDismissalKept() {
|
||||
when(signalService.userFrequency(eq(PRINCIPAL), anyLong(), anyLong()))
|
||||
.thenReturn(Map.of("ocr", 6.0));
|
||||
when(dismissalRepository.findByPrincipal(PRINCIPAL))
|
||||
.thenReturn(
|
||||
List.of(new ToolRecommendationDismissal(PRINCIPAL, "merge", "ocr")));
|
||||
|
||||
List<ToolRecommendation> result = service.getRecommendations(PRINCIPAL, "compare", 6);
|
||||
|
||||
assertThat(toolKeys(result)).containsExactly("ocr");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("an any-context dismissal hides the tool everywhere")
|
||||
void anyContextDismissalFilters() {
|
||||
when(signalService.userFrequency(eq(PRINCIPAL), anyLong(), anyLong()))
|
||||
.thenReturn(Map.of("ocr", 6.0));
|
||||
when(dismissalRepository.findByPrincipal(PRINCIPAL))
|
||||
.thenReturn(
|
||||
List.of(
|
||||
new ToolRecommendationDismissal(
|
||||
PRINCIPAL,
|
||||
ToolRecommendationDismissal.ANY_CONTEXT,
|
||||
"ocr")));
|
||||
|
||||
assertThat(service.getRecommendations(PRINCIPAL, null, 6)).isEmpty();
|
||||
assertThat(service.getRecommendations(PRINCIPAL, "compare", 6)).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("a dismissal takes effect on the very next read (nothing cached)")
|
||||
void dismissalAppliesImmediately() {
|
||||
when(signalService.userFrequency(eq(PRINCIPAL), anyLong(), anyLong()))
|
||||
.thenReturn(Map.of("ocr", 6.0, "merge", 5.0));
|
||||
when(dismissalRepository.findByPrincipal(PRINCIPAL))
|
||||
.thenReturn(List.of())
|
||||
.thenReturn(
|
||||
List.of(new ToolRecommendationDismissal(PRINCIPAL, "compare", "ocr")));
|
||||
|
||||
assertThat(toolKeys(service.getRecommendations(PRINCIPAL, "compare", 6)))
|
||||
.containsExactly("ocr", "merge");
|
||||
assertThat(toolKeys(service.getRecommendations(PRINCIPAL, "compare", 6)))
|
||||
.containsExactly("merge");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("dismiss saves the row keyed by principal, context and tool")
|
||||
void dismissSavesRow() {
|
||||
service.dismiss(PRINCIPAL, "compare", "ocr");
|
||||
|
||||
ArgumentCaptor<ToolRecommendationDismissal> captor =
|
||||
ArgumentCaptor.forClass(ToolRecommendationDismissal.class);
|
||||
verify(dismissalRepository).save(captor.capture());
|
||||
assertThat(captor.getValue().getPrincipal()).isEqualTo(PRINCIPAL);
|
||||
assertThat(captor.getValue().getContextTool()).isEqualTo("compare");
|
||||
assertThat(captor.getValue().getDismissedTool()).isEqualTo("ocr");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("undoDismiss deletes the stored dismissal")
|
||||
void undoDismissDeletes() {
|
||||
ToolRecommendationDismissal stored =
|
||||
new ToolRecommendationDismissal(PRINCIPAL, "compare", "ocr");
|
||||
when(dismissalRepository.findById(
|
||||
new ToolRecommendationDismissalId(PRINCIPAL, "compare", "ocr")))
|
||||
.thenReturn(Optional.of(stored));
|
||||
|
||||
service.undoDismiss(PRINCIPAL, "compare", "ocr");
|
||||
|
||||
verify(dismissalRepository).delete(stored);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("undoing a dismissal that was never made is a no-op")
|
||||
void undoUnknownDismissalIsNoOp() {
|
||||
when(dismissalRepository.findById(
|
||||
new ToolRecommendationDismissalId(PRINCIPAL, "compare", "ocr")))
|
||||
.thenReturn(Optional.empty());
|
||||
|
||||
service.undoDismiss(PRINCIPAL, "compare", "ocr");
|
||||
|
||||
verify(dismissalRepository, never()).delete(any(ToolRecommendationDismissal.class));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
-24
@@ -28,10 +28,8 @@ import org.testcontainers.junit.jupiter.Testcontainers;
|
||||
|
||||
import stirling.software.common.model.ApplicationProperties;
|
||||
import stirling.software.proprietary.model.ToolChainStat;
|
||||
import stirling.software.proprietary.model.ToolRecommendationDismissal;
|
||||
import stirling.software.proprietary.model.ToolUsageStat;
|
||||
import stirling.software.proprietary.repository.ToolChainStatRepository;
|
||||
import stirling.software.proprietary.repository.ToolRecommendationDismissalRepository;
|
||||
import stirling.software.proprietary.repository.ToolUsageStatRepository;
|
||||
|
||||
/**
|
||||
@@ -69,10 +67,8 @@ class ToolUsagePostgresConcurrencyTest {
|
||||
|
||||
@Autowired private ToolUsageStatRepository usageRepository;
|
||||
@Autowired private ToolChainStatRepository chainRepository;
|
||||
@Autowired private ToolRecommendationDismissalRepository dismissalRepository;
|
||||
|
||||
private ToolUsageTrackingService trackingService;
|
||||
private ToolRecommendationService recommendationService;
|
||||
|
||||
@BeforeEach
|
||||
void setUp() {
|
||||
@@ -80,12 +76,6 @@ class ToolUsagePostgresConcurrencyTest {
|
||||
properties.getSystem().setEnableAnalytics(true);
|
||||
trackingService =
|
||||
new ToolUsageTrackingService(usageRepository, chainRepository, properties);
|
||||
recommendationService =
|
||||
new ToolRecommendationService(
|
||||
new ToolUsageSignalService(
|
||||
usageRepository, chainRepository, java.util.Optional.empty()),
|
||||
dismissalRepository,
|
||||
properties);
|
||||
}
|
||||
|
||||
/** Runs {@code task} on {@code NODES} threads at once; returns how many threw. */
|
||||
@@ -189,20 +179,6 @@ class ToolUsagePostgresConcurrencyTest {
|
||||
assertThat(rows.get(0).getChainLength()).isEqualTo(2);
|
||||
}
|
||||
|
||||
@Test
|
||||
@Transactional(propagation = Propagation.NOT_SUPPORTED)
|
||||
@DisplayName("the same dismissal from several nodes at once stores one row and never throws")
|
||||
void concurrentDismissalsAreIdempotent() throws InterruptedException {
|
||||
dismissalRepository.deleteAll();
|
||||
|
||||
int failures = race(NODES, () -> recommendationService.dismiss("alice", "compare", "ocr"));
|
||||
|
||||
assertThat(failures).isZero();
|
||||
assertThat(dismissalRepository.findByPrincipal("alice"))
|
||||
.extracting(ToolRecommendationDismissal::getDismissedTool)
|
||||
.containsExactly("ocr");
|
||||
}
|
||||
|
||||
@Test
|
||||
@Transactional(propagation = Propagation.NOT_SUPPORTED)
|
||||
@DisplayName("the retention sweep is idempotent when every node runs it together")
|
||||
|
||||
@@ -10828,12 +10828,6 @@ noToolsFound = "No tools found"
|
||||
quickAccess = "QUICK ACCESS"
|
||||
searchPlaceholder = "Search tools..."
|
||||
|
||||
[toolPicker.recommendations]
|
||||
dismiss = "Don't recommend this tool here"
|
||||
dismissed = "{{tool}} won't be recommended here again"
|
||||
dismissFailed = "Could not save that preference. Please try again."
|
||||
undo = "Undo"
|
||||
|
||||
[toolPicker.subcategories]
|
||||
advancedFormatting = "Advanced Formatting"
|
||||
automation = "Automation"
|
||||
|
||||
@@ -10953,12 +10953,6 @@ noToolsFound = "No tools found"
|
||||
quickAccess = "QUICK ACCESS"
|
||||
searchPlaceholder = "Search tools..."
|
||||
|
||||
[toolPicker.recommendations]
|
||||
dismiss = "Don't recommend this tool here"
|
||||
dismissed = "{{tool}} won't be recommended here again"
|
||||
dismissFailed = "Could not save that preference. Please try again."
|
||||
undo = "Undo"
|
||||
|
||||
[toolPicker.subcategories]
|
||||
advancedFormatting = "Advanced Formatting"
|
||||
automation = "Automation"
|
||||
|
||||
@@ -5,18 +5,15 @@ import {
|
||||
fetchToolRecommendations,
|
||||
fetchToolWorkflows,
|
||||
recordToolUsage,
|
||||
dismissToolRecommendation,
|
||||
undoDismissToolRecommendation,
|
||||
resetToolRecommendationsAvailabilityForTests,
|
||||
} from "@app/api/toolRecommendations";
|
||||
|
||||
vi.mock("@app/services/apiClient", () => ({
|
||||
default: { get: vi.fn(), post: vi.fn(), delete: vi.fn() },
|
||||
default: { get: vi.fn(), post: vi.fn() },
|
||||
}));
|
||||
|
||||
const mockGet = vi.mocked(apiClient.get);
|
||||
const mockPost = vi.mocked(apiClient.post);
|
||||
const mockDelete = vi.mocked(apiClient.delete);
|
||||
|
||||
const http404 = Object.assign(new Error("not found"), {
|
||||
response: { status: 404 },
|
||||
@@ -140,47 +137,4 @@ describe("toolRecommendations api", () => {
|
||||
expect(mockPost).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("dismissals", () => {
|
||||
it("posts a context-scoped dismissal", async () => {
|
||||
mockPost.mockResolvedValue({});
|
||||
|
||||
await dismissToolRecommendation("compare", "ocr");
|
||||
|
||||
expect(mockPost).toHaveBeenCalledWith(
|
||||
expect.stringContaining("/dismissals"),
|
||||
{ contextTool: "compare", dismissedTool: "ocr" },
|
||||
expect.objectContaining({ suppressErrorToast: true }),
|
||||
);
|
||||
});
|
||||
|
||||
it("maps a null context to the any-context wildcard", async () => {
|
||||
mockPost.mockResolvedValue({});
|
||||
|
||||
await dismissToolRecommendation(null, "ocr");
|
||||
|
||||
expect(mockPost.mock.calls[0][1]).toEqual({
|
||||
contextTool: "*",
|
||||
dismissedTool: "ocr",
|
||||
});
|
||||
});
|
||||
|
||||
it("undo issues a delete with the same coordinates", async () => {
|
||||
mockDelete.mockResolvedValue({});
|
||||
|
||||
await undoDismissToolRecommendation("compare", "ocr");
|
||||
|
||||
const url = mockDelete.mock.calls[0][0] as string;
|
||||
expect(url).toContain("contextTool=compare");
|
||||
expect(url).toContain("dismissedTool=ocr");
|
||||
});
|
||||
|
||||
it("propagates dismissal failures so the UI can warn the user", async () => {
|
||||
mockPost.mockRejectedValue(new Error("boom"));
|
||||
|
||||
await expect(dismissToolRecommendation("compare", "ocr")).rejects.toThrow(
|
||||
"boom",
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,9 +5,6 @@ export interface ToolRecommendationDto {
|
||||
score: number;
|
||||
}
|
||||
|
||||
/** Dismiss in every context (used when no tool is active). */
|
||||
export const ANY_CONTEXT = "*";
|
||||
|
||||
const BASE_PATH = "/api/v1/proprietary/ui-data/tool-recommendations";
|
||||
|
||||
// Core-only backends have no recommendations API; remember the 404 so we stop asking.
|
||||
@@ -106,28 +103,3 @@ export async function fetchToolWorkflows(
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
export async function dismissToolRecommendation(
|
||||
contextTool: string | null,
|
||||
dismissedTool: string,
|
||||
): Promise<void> {
|
||||
await apiClient.post(
|
||||
`${BASE_PATH}/dismissals`,
|
||||
{ contextTool: contextTool ?? ANY_CONTEXT, dismissedTool },
|
||||
{ suppressErrorToast: true, skipAuthRedirect: true },
|
||||
);
|
||||
}
|
||||
|
||||
export async function undoDismissToolRecommendation(
|
||||
contextTool: string | null,
|
||||
dismissedTool: string,
|
||||
): Promise<void> {
|
||||
const params = new URLSearchParams({
|
||||
contextTool: contextTool ?? ANY_CONTEXT,
|
||||
dismissedTool,
|
||||
});
|
||||
await apiClient.delete(`${BASE_PATH}/dismissals?${params}`, {
|
||||
suppressErrorToast: true,
|
||||
skipAuthRedirect: true,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,16 +1,11 @@
|
||||
import React, { useCallback, useMemo, useRef } from "react";
|
||||
import React, { useMemo, useRef } from "react";
|
||||
import { Box, Stack } from "@mantine/core";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { alert } from "@app/components/toast";
|
||||
import { Button } from "@app/ui/Button";
|
||||
import { ToolRegistryEntry } from "@app/data/toolsTaxonomy";
|
||||
import "@app/components/tools/toolPicker/ToolPicker.css";
|
||||
import { useToolSections } from "@app/hooks/useToolSections";
|
||||
import type { SubcategoryGroup } from "@app/hooks/useToolSections";
|
||||
import {
|
||||
useDismissToolRecommendation,
|
||||
useRecommendationContextTool,
|
||||
} from "@app/hooks/useToolRecommendations";
|
||||
import { useFavoriteToolItems } from "@app/hooks/tools/useFavoriteToolItems";
|
||||
import NoToolsFound from "@app/components/tools/shared/NoToolsFound";
|
||||
import { renderToolButtons } from "@app/components/tools/shared/renderToolButtons";
|
||||
@@ -75,57 +70,8 @@ const ToolPicker = ({
|
||||
|
||||
const scrollableRef = useRef<HTMLDivElement>(null);
|
||||
|
||||
const { sections: visibleSections, rankedRecommendationIds } =
|
||||
useToolSections(filteredTools);
|
||||
const { sections: visibleSections } = useToolSections(filteredTools);
|
||||
const { favoriteTools, toolRegistry } = useToolWorkflowData();
|
||||
const recommendationContext = useRecommendationContextTool();
|
||||
const dismissRecommendation = useDismissToolRecommendation();
|
||||
|
||||
// Dismiss only applies to usage-derived recommendations; the static list is not persisted.
|
||||
const handleDismissRecommendation = useCallback(
|
||||
(toolId: ToolId, toolName: string) => {
|
||||
const reportFailure = () =>
|
||||
alert({
|
||||
alertType: "error",
|
||||
title: t(
|
||||
"toolPicker.recommendations.dismissFailed",
|
||||
"Could not save that preference. Please try again.",
|
||||
),
|
||||
});
|
||||
|
||||
void (async () => {
|
||||
try {
|
||||
const undo = await dismissRecommendation(
|
||||
recommendationContext,
|
||||
toolId,
|
||||
);
|
||||
alert({
|
||||
alertType: "neutral",
|
||||
title: t("toolPicker.recommendations.dismissed", {
|
||||
defaultValue: "{{tool}} won't be recommended here again",
|
||||
tool: toolName,
|
||||
}),
|
||||
buttonText: t("toolPicker.recommendations.undo", "Undo"),
|
||||
buttonCallback: () => void undo().catch(reportFailure),
|
||||
durationMs: 6000,
|
||||
});
|
||||
} catch {
|
||||
reportFailure();
|
||||
}
|
||||
})();
|
||||
},
|
||||
[dismissRecommendation, recommendationContext, t],
|
||||
);
|
||||
|
||||
// Only usage-ranked entries can be dismissed; on the curated top-up (and on
|
||||
// Shared Signing, pinned by its badge) a dismissal could never take effect.
|
||||
const dismissHandlerFor = useCallback(
|
||||
(id: string, tool: ToolRegistryEntry) =>
|
||||
rankedRecommendationIds.has(id as ToolId) && id !== "sharedSign"
|
||||
? () => handleDismissRecommendation(id as ToolId, tool.name)
|
||||
: undefined,
|
||||
[rankedRecommendationIds, handleDismissRecommendation],
|
||||
);
|
||||
|
||||
const favoriteToolItems = useFavoriteToolItems(favoriteTools, toolRegistry);
|
||||
|
||||
@@ -231,7 +177,6 @@ const ToolPicker = ({
|
||||
badgeCount={
|
||||
id === "sharedSign" ? signingBadgeCount : undefined
|
||||
}
|
||||
onDismiss={dismissHandlerFor(id, tool)}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
@@ -289,7 +234,6 @@ const ToolPicker = ({
|
||||
badgeCount={
|
||||
id === "sharedSign" ? signingBadgeCount : undefined
|
||||
}
|
||||
onDismiss={dismissHandlerFor(id, tool)}
|
||||
/>
|
||||
))}
|
||||
</div>
|
||||
|
||||
@@ -17,26 +17,24 @@ export function SuggestedToolsSection(): React.ReactElement {
|
||||
</Text>
|
||||
|
||||
<Stack gap="xs">
|
||||
{suggestedTools.map((tool) => {
|
||||
const IconComponent = tool.icon;
|
||||
return (
|
||||
<Anchor
|
||||
key={tool.id}
|
||||
href={tool.href}
|
||||
onClick={tool.onClick}
|
||||
style={{ textDecoration: "none", color: "inherit" }}
|
||||
>
|
||||
<Card p="sm" withBorder style={{ cursor: "pointer" }}>
|
||||
<Group gap="xs">
|
||||
<ToolIcon icon={<IconComponent />} />
|
||||
<Text size="sm" fw={500}>
|
||||
{tool.title}
|
||||
</Text>
|
||||
</Group>
|
||||
</Card>
|
||||
</Anchor>
|
||||
);
|
||||
})}
|
||||
{suggestedTools.map((tool) => (
|
||||
<Anchor
|
||||
key={tool.id}
|
||||
href={tool.href}
|
||||
onClick={tool.onClick}
|
||||
data-tour={`suggested-tool-${tool.id}`}
|
||||
style={{ textDecoration: "none", color: "inherit" }}
|
||||
>
|
||||
<Card p="sm" withBorder style={{ cursor: "pointer" }}>
|
||||
<Group gap="xs">
|
||||
<ToolIcon icon={tool.icon} />
|
||||
<Text size="sm" fw={500}>
|
||||
{tool.title}
|
||||
</Text>
|
||||
</Group>
|
||||
</Card>
|
||||
</Anchor>
|
||||
))}
|
||||
</Stack>
|
||||
</Stack>
|
||||
);
|
||||
|
||||
@@ -1,125 +0,0 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { render, screen, fireEvent } from "@testing-library/react";
|
||||
import { MantineProvider } from "@mantine/core";
|
||||
|
||||
import ToolButton from "@app/components/tools/toolPicker/ToolButton";
|
||||
import {
|
||||
SubcategoryId,
|
||||
ToolCategoryId,
|
||||
ToolRegistryEntry,
|
||||
} from "@app/data/toolsTaxonomy";
|
||||
|
||||
vi.mock("@app/contexts/ToolWorkflowContext", () => ({
|
||||
useToolWorkflowData: () => ({
|
||||
isFavorite: () => false,
|
||||
toolAvailability: {},
|
||||
}),
|
||||
useToolWorkflowActions: () => ({ toggleFavorite: vi.fn() }),
|
||||
}));
|
||||
vi.mock("@app/contexts/HotkeyContext", () => ({
|
||||
useHotkeys: () => ({ hotkeys: {} }),
|
||||
}));
|
||||
vi.mock("@app/hooks/useToolNavigation", () => ({
|
||||
useToolNavigation: () => ({ getToolNavigation: () => null }),
|
||||
}));
|
||||
vi.mock("@app/contexts/AppConfigContext", () => ({
|
||||
useAppConfig: () => ({ config: {} }),
|
||||
}));
|
||||
vi.mock("@app/hooks/useWillUseCloud", () => ({
|
||||
useWillUseCloud: () => false,
|
||||
}));
|
||||
// Tooltip pulls in preferences/logo providers irrelevant to this test.
|
||||
vi.mock("@app/components/shared/Tooltip", () => ({
|
||||
Tooltip: ({ children }: { children: React.ReactNode }) => <>{children}</>,
|
||||
}));
|
||||
|
||||
const tool: ToolRegistryEntry = {
|
||||
icon: null,
|
||||
name: "OCR",
|
||||
component: (() => null) as never,
|
||||
description: "Recognise text",
|
||||
categoryId: ToolCategoryId.RECOMMENDED_TOOLS,
|
||||
subcategoryId: SubcategoryId.EXTRACTION,
|
||||
automationSettings: null,
|
||||
} as ToolRegistryEntry;
|
||||
|
||||
const renderButton = (props: Partial<Parameters<typeof ToolButton>[0]> = {}) =>
|
||||
render(
|
||||
<MantineProvider>
|
||||
<ToolButton
|
||||
id={"ocr" as never}
|
||||
tool={tool}
|
||||
isSelected={false}
|
||||
onSelect={vi.fn()}
|
||||
{...props}
|
||||
/>
|
||||
</MantineProvider>,
|
||||
);
|
||||
|
||||
describe("ToolButton dismiss control", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("renders no dismiss control by default", () => {
|
||||
renderButton();
|
||||
|
||||
expect(
|
||||
screen.queryByLabelText("toolPicker.recommendations.dismiss"),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("renders the dismiss X when onDismiss is provided", () => {
|
||||
renderButton({ onDismiss: vi.fn() });
|
||||
|
||||
expect(
|
||||
screen.getByLabelText("toolPicker.recommendations.dismiss"),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("clicking the X fires onDismiss without selecting the tool", () => {
|
||||
const onDismiss = vi.fn();
|
||||
const onSelect = vi.fn();
|
||||
renderButton({ onDismiss, onSelect });
|
||||
|
||||
fireEvent.click(
|
||||
screen.getByLabelText("toolPicker.recommendations.dismiss"),
|
||||
);
|
||||
|
||||
expect(onDismiss).toHaveBeenCalledTimes(1);
|
||||
expect(onSelect).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("clicking the tool itself still selects it", () => {
|
||||
const onSelect = vi.fn();
|
||||
renderButton({ onDismiss: vi.fn(), onSelect });
|
||||
|
||||
fireEvent.click(screen.getByText("OCR"));
|
||||
|
||||
expect(onSelect).toHaveBeenCalledWith("ocr");
|
||||
});
|
||||
|
||||
it("exposes the X as a keyboard-reachable button to assistive tech", () => {
|
||||
renderButton({ onDismiss: vi.fn() });
|
||||
|
||||
// aria-label is ARIA-prohibited on a bare span, so role is what makes it nameable.
|
||||
const dismissButton = screen.getByRole("button", {
|
||||
name: "toolPicker.recommendations.dismiss",
|
||||
});
|
||||
expect(dismissButton).toHaveAttribute("tabindex", "0");
|
||||
});
|
||||
|
||||
it.each(["Enter", " "])("pressing %s on the X fires onDismiss", (key) => {
|
||||
const onDismiss = vi.fn();
|
||||
const onSelect = vi.fn();
|
||||
renderButton({ onDismiss, onSelect });
|
||||
|
||||
fireEvent.keyDown(
|
||||
screen.getByLabelText("toolPicker.recommendations.dismiss"),
|
||||
{ key },
|
||||
);
|
||||
|
||||
expect(onDismiss).toHaveBeenCalledTimes(1);
|
||||
expect(onSelect).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -1,7 +1,5 @@
|
||||
import React, { memo } from "react";
|
||||
import { Badge } from "@mantine/core";
|
||||
import CloseRoundedIcon from "@mui/icons-material/CloseRounded";
|
||||
import { ActionIcon } from "@app/ui/ActionIcon";
|
||||
import { Button } from "@app/ui/Button";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { Tooltip } from "@app/components/shared/Tooltip";
|
||||
@@ -39,8 +37,6 @@ interface ToolButtonProps {
|
||||
/** Called when an unavailable tool is clicked; if provided, overrides the default no-op */
|
||||
onUnavailableClick?: () => void;
|
||||
badgeCount?: number;
|
||||
/** Shows a hover-only X that dismisses this tool from the recommended list. */
|
||||
onDismiss?: () => void;
|
||||
}
|
||||
|
||||
const ToolButton: React.FC<ToolButtonProps> = ({
|
||||
@@ -54,7 +50,6 @@ const ToolButton: React.FC<ToolButtonProps> = ({
|
||||
showDescription = false,
|
||||
onUnavailableClick,
|
||||
badgeCount,
|
||||
onDismiss,
|
||||
}) => {
|
||||
const { t } = useTranslation();
|
||||
const { config } = useAppConfig();
|
||||
@@ -319,46 +314,9 @@ const ToolButton: React.FC<ToolButtonProps> = ({
|
||||
/>
|
||||
) : null;
|
||||
|
||||
const handleDismiss = (e: React.SyntheticEvent) => {
|
||||
e.stopPropagation();
|
||||
e.preventDefault();
|
||||
onDismiss?.();
|
||||
};
|
||||
|
||||
const dismiss = onDismiss ? (
|
||||
<ActionIcon
|
||||
// A span (no control nesting); role="button" makes the aria-label legal
|
||||
// and tabIndex keeps it keyboard-reachable.
|
||||
as="span"
|
||||
role="button"
|
||||
tabIndex={0}
|
||||
variant="tertiary"
|
||||
shape="circle"
|
||||
size="sm"
|
||||
onClick={handleDismiss}
|
||||
onKeyDown={(e: React.KeyboardEvent) => {
|
||||
e.stopPropagation();
|
||||
if (e.key === "Enter" || e.key === " ") handleDismiss(e);
|
||||
}}
|
||||
onMouseDown={(e: React.MouseEvent) => e.stopPropagation()}
|
||||
className="tool-button-dismiss"
|
||||
aria-label={t(
|
||||
"toolPicker.recommendations.dismiss",
|
||||
"Don't recommend this tool here",
|
||||
)}
|
||||
title={t(
|
||||
"toolPicker.recommendations.dismiss",
|
||||
"Don't recommend this tool here",
|
||||
)}
|
||||
>
|
||||
<CloseRoundedIcon fontSize="inherit" style={{ fontSize: "1rem" }} />
|
||||
</ActionIcon>
|
||||
) : null;
|
||||
|
||||
return (
|
||||
<div className="tool-button-container">
|
||||
{star}
|
||||
{dismiss}
|
||||
<Tooltip
|
||||
content={tooltipContent}
|
||||
position="left"
|
||||
|
||||
@@ -103,23 +103,6 @@
|
||||
opacity: 1;
|
||||
}
|
||||
|
||||
/* Hover-only dismiss X for recommended tools; sits left of the favourite star. */
|
||||
.tool-button-dismiss {
|
||||
position: absolute;
|
||||
top: 50%;
|
||||
transform: translateY(-50%);
|
||||
right: 2rem;
|
||||
opacity: 0;
|
||||
transition: opacity 0.2s ease;
|
||||
z-index: var(--z-toolpicker-star);
|
||||
}
|
||||
|
||||
/* :focus-visible too, or the keyboard-reachable control stays invisible while focused. */
|
||||
.tool-button-container:hover .tool-button-dismiss,
|
||||
.tool-button-dismiss:focus-visible {
|
||||
opacity: 1;
|
||||
}
|
||||
|
||||
.search-input-container {
|
||||
margin-top: 0.5rem;
|
||||
margin-bottom: 0.5rem;
|
||||
|
||||
@@ -0,0 +1,116 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { renderHook } from "@testing-library/react";
|
||||
|
||||
import { useSuggestedTools } from "@app/hooks/useSuggestedTools";
|
||||
import { useToolRecommendations } from "@app/hooks/useToolRecommendations";
|
||||
import { useNavigationState } from "@app/contexts/NavigationContext";
|
||||
import { useToolWorkflow } from "@app/contexts/ToolWorkflowContext";
|
||||
import type { ToolId } from "@app/types/toolId";
|
||||
import type { ToolRegistryEntry } from "@app/data/toolsTaxonomy";
|
||||
|
||||
vi.mock("@app/hooks/useToolRecommendations", () => ({
|
||||
useToolRecommendations: vi.fn(),
|
||||
}));
|
||||
vi.mock("@app/contexts/NavigationContext", () => ({
|
||||
useNavigationState: vi.fn(),
|
||||
}));
|
||||
vi.mock("@app/contexts/ToolWorkflowContext", () => ({
|
||||
useToolWorkflow: vi.fn(),
|
||||
}));
|
||||
vi.mock("@app/hooks/useToolNavigation", () => ({
|
||||
useToolNavigation: () => ({
|
||||
getToolNavigation: (toolId: string) => ({
|
||||
href: `/${toolId}`,
|
||||
onClick: () => {},
|
||||
}),
|
||||
}),
|
||||
}));
|
||||
|
||||
const mockRecommendations = vi.mocked(useToolRecommendations);
|
||||
const mockNavigation = vi.mocked(useNavigationState);
|
||||
const mockWorkflow = vi.mocked(useToolWorkflow);
|
||||
|
||||
/** A tool that can actually open, so it survives the availability filter. */
|
||||
function entry(name: string): ToolRegistryEntry {
|
||||
return {
|
||||
name,
|
||||
icon: null,
|
||||
component: (() => null) as unknown as ToolRegistryEntry["component"],
|
||||
} as ToolRegistryEntry;
|
||||
}
|
||||
|
||||
const REGISTRY: Partial<Record<ToolId, ToolRegistryEntry>> = {
|
||||
compress: entry("Compress"),
|
||||
convert: entry("Convert"),
|
||||
sanitize: entry("Sanitize"),
|
||||
split: entry("Split"),
|
||||
ocr: entry("OCR"),
|
||||
addPassword: entry("Add Password"),
|
||||
merge: entry("Merge"),
|
||||
// No component and no link - nothing to open.
|
||||
automate: {
|
||||
name: "Automate",
|
||||
icon: null,
|
||||
component: null,
|
||||
} as ToolRegistryEntry,
|
||||
};
|
||||
|
||||
function setup(
|
||||
recommendedToolIds: ToolId[] | null,
|
||||
selectedTool: ToolId | null = null,
|
||||
) {
|
||||
mockRecommendations.mockReturnValue({
|
||||
recommendedToolIds,
|
||||
contextTool: selectedTool,
|
||||
});
|
||||
mockNavigation.mockReturnValue({ selectedTool } as ReturnType<
|
||||
typeof useNavigationState
|
||||
>);
|
||||
mockWorkflow.mockReturnValue({
|
||||
getSelectedTool: (id: ToolId | null) =>
|
||||
id ? (REGISTRY[id] ?? null) : null,
|
||||
} as unknown as ReturnType<typeof useToolWorkflow>);
|
||||
|
||||
return renderHook(() => useSuggestedTools()).result.current.map((t) => t.id);
|
||||
}
|
||||
|
||||
describe("useSuggestedTools", () => {
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
it("shows the curated list when the backend has no usage data", () => {
|
||||
expect(setup(null)).toEqual(["compress", "convert", "sanitize", "split"]);
|
||||
});
|
||||
|
||||
it("leads with the usage ranking, then tops up from the curated list", () => {
|
||||
expect(setup(["addPassword", "merge"])).toEqual([
|
||||
"addPassword",
|
||||
"merge",
|
||||
"compress",
|
||||
"convert",
|
||||
]);
|
||||
});
|
||||
|
||||
it("never suggests the tool the user is currently in", () => {
|
||||
expect(setup(["compress", "addPassword"], "compress")).toEqual([
|
||||
"addPassword",
|
||||
"convert",
|
||||
"sanitize",
|
||||
"split",
|
||||
]);
|
||||
});
|
||||
|
||||
it("skips tools that are unknown or cannot open", () => {
|
||||
expect(setup(["automate", "nonsense" as ToolId, "merge"])).toEqual([
|
||||
"merge",
|
||||
"compress",
|
||||
"convert",
|
||||
"sanitize",
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not repeat a ranked tool that is also in the curated list", () => {
|
||||
const ids = setup(["split", "compress"]);
|
||||
expect(ids).toEqual(["split", "compress", "convert", "sanitize"]);
|
||||
expect(new Set(ids).size).toBe(ids.length);
|
||||
});
|
||||
});
|
||||
@@ -2,81 +2,66 @@ import { useMemo } from "react";
|
||||
import { useNavigationState } from "@app/contexts/NavigationContext";
|
||||
import { useToolNavigation } from "@app/hooks/useToolNavigation";
|
||||
import { useToolWorkflow } from "@app/contexts/ToolWorkflowContext";
|
||||
import { useToolRecommendations } from "@app/hooks/useToolRecommendations";
|
||||
import { ToolId } from "@app/types/toolId";
|
||||
|
||||
// Material UI Icons
|
||||
import CompressIcon from "@mui/icons-material/Compress";
|
||||
import SwapHorizIcon from "@mui/icons-material/SwapHoriz";
|
||||
import CleaningServicesIcon from "@mui/icons-material/CleaningServices";
|
||||
import CropIcon from "@mui/icons-material/Crop";
|
||||
import TextFieldsIcon from "@mui/icons-material/TextFields";
|
||||
|
||||
export interface SuggestedTool {
|
||||
id: ToolId;
|
||||
title: string;
|
||||
icon: React.ComponentType<any>;
|
||||
icon: React.ReactNode;
|
||||
href: string;
|
||||
onClick: (e: React.MouseEvent) => void;
|
||||
}
|
||||
|
||||
const ALL_SUGGESTED_TOOLS: Omit<SuggestedTool, "href" | "onClick">[] = [
|
||||
{
|
||||
id: "compress",
|
||||
title: "Compress",
|
||||
icon: CompressIcon,
|
||||
},
|
||||
{
|
||||
id: "convert",
|
||||
title: "Convert",
|
||||
icon: SwapHorizIcon,
|
||||
},
|
||||
{
|
||||
id: "sanitize",
|
||||
title: "Sanitize",
|
||||
icon: CleaningServicesIcon,
|
||||
},
|
||||
{
|
||||
id: "split",
|
||||
title: "Split",
|
||||
icon: CropIcon,
|
||||
},
|
||||
{
|
||||
id: "ocr",
|
||||
title: "OCR",
|
||||
icon: TextFieldsIcon,
|
||||
},
|
||||
/** Shown when usage tracking is off or has nothing to say yet. */
|
||||
const FALLBACK_TOOL_IDS: ToolId[] = [
|
||||
"compress",
|
||||
"convert",
|
||||
"sanitize",
|
||||
"split",
|
||||
"ocr",
|
||||
];
|
||||
|
||||
const SUGGESTION_COUNT = 4;
|
||||
|
||||
// A couple spare, so tools that cannot open still leave a full list.
|
||||
const FETCH_LIMIT = SUGGESTION_COUNT + 2;
|
||||
|
||||
/**
|
||||
* What to do next with the file that just came out of a tool.
|
||||
*
|
||||
* Ranked by how this user, their team and the install actually use tools after
|
||||
* the current one, and topped up from the curated list so the section never
|
||||
* shrinks. Falls back to the curated list entirely when the backend has no
|
||||
* usage data - a fresh install, or analytics turned off.
|
||||
*/
|
||||
export function useSuggestedTools(): SuggestedTool[] {
|
||||
const { selectedTool } = useNavigationState();
|
||||
const { getToolNavigation } = useToolNavigation();
|
||||
const { getSelectedTool } = useToolWorkflow();
|
||||
const { recommendedToolIds } = useToolRecommendations(FETCH_LIMIT);
|
||||
|
||||
return useMemo(() => {
|
||||
// Filter out the current tool
|
||||
const filteredTools = ALL_SUGGESTED_TOOLS.filter(
|
||||
(tool) => tool.id !== selectedTool,
|
||||
);
|
||||
const ordered = [...(recommendedToolIds ?? []), ...FALLBACK_TOOL_IDS];
|
||||
const suggestions: SuggestedTool[] = [];
|
||||
const seen = new Set<ToolId>();
|
||||
|
||||
// Add navigation props to each tool
|
||||
return filteredTools.map((tool) => {
|
||||
const toolRegistryEntry = getSelectedTool(tool.id);
|
||||
if (!toolRegistryEntry) {
|
||||
// Fallback for tools not in registry
|
||||
return {
|
||||
...tool,
|
||||
href: `/${tool.id}`,
|
||||
onClick: (e: React.MouseEvent) => {
|
||||
e.preventDefault();
|
||||
},
|
||||
};
|
||||
}
|
||||
for (const id of ordered) {
|
||||
if (id === selectedTool || seen.has(id)) continue;
|
||||
const tool = getSelectedTool(id);
|
||||
// A card that cannot open anything is worse than a shorter list.
|
||||
if (!tool || (tool.component === null && !tool.link)) continue;
|
||||
|
||||
const navProps = getToolNavigation(tool.id, toolRegistryEntry);
|
||||
return {
|
||||
...tool,
|
||||
...navProps,
|
||||
};
|
||||
});
|
||||
}, [selectedTool, getToolNavigation, getSelectedTool]);
|
||||
seen.add(id);
|
||||
suggestions.push({
|
||||
id,
|
||||
title: tool.name,
|
||||
icon: tool.icon,
|
||||
...getToolNavigation(id, tool),
|
||||
});
|
||||
if (suggestions.length === SUGGESTION_COUNT) break;
|
||||
}
|
||||
|
||||
return suggestions;
|
||||
}, [recommendedToolIds, selectedTool, getToolNavigation, getSelectedTool]);
|
||||
}
|
||||
|
||||
@@ -1,34 +1,20 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { renderHook, waitFor, act } from "@testing-library/react";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
|
||||
import { qk } from "@app/query/keys";
|
||||
import { TestQueryProvider } from "@app/tests/utils/TestQueryProvider";
|
||||
import {
|
||||
fetchToolRecommendations,
|
||||
dismissToolRecommendation,
|
||||
undoDismissToolRecommendation,
|
||||
} from "@app/api/toolRecommendations";
|
||||
import {
|
||||
useDismissToolRecommendation,
|
||||
useToolRecommendations,
|
||||
} from "@app/hooks/useToolRecommendations";
|
||||
import { fetchToolRecommendations } from "@app/api/toolRecommendations";
|
||||
import { useToolRecommendations } from "@app/hooks/useToolRecommendations";
|
||||
import {
|
||||
notifyToolCompleted,
|
||||
resetToolUsageTrackerForTests,
|
||||
} from "@app/services/toolUsageTracker";
|
||||
|
||||
vi.mock("@app/api/toolRecommendations", () => ({
|
||||
ANY_CONTEXT: "*",
|
||||
fetchToolRecommendations: vi.fn(),
|
||||
recordToolUsage: vi.fn().mockResolvedValue(undefined),
|
||||
dismissToolRecommendation: vi.fn(),
|
||||
undoDismissToolRecommendation: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockFetch = vi.mocked(fetchToolRecommendations);
|
||||
const mockDismiss = vi.mocked(dismissToolRecommendation);
|
||||
const mockUndo = vi.mocked(undoDismissToolRecommendation);
|
||||
|
||||
describe("useToolRecommendations", () => {
|
||||
beforeEach(() => {
|
||||
@@ -94,89 +80,3 @@ describe("useToolRecommendations", () => {
|
||||
expect(result.current.contextTool).toBe("compare");
|
||||
});
|
||||
});
|
||||
|
||||
describe("useDismissToolRecommendation", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
resetToolUsageTrackerForTests();
|
||||
});
|
||||
|
||||
it("persists the dismissal and returns a working undo", async () => {
|
||||
mockDismiss.mockResolvedValue(undefined);
|
||||
mockUndo.mockResolvedValue(undefined);
|
||||
|
||||
const { result } = renderHook(() => useDismissToolRecommendation(), {
|
||||
wrapper: TestQueryProvider,
|
||||
});
|
||||
|
||||
const undo = await result.current("compare", "ocr");
|
||||
expect(mockDismiss).toHaveBeenCalledWith("compare", "ocr");
|
||||
|
||||
await undo();
|
||||
expect(mockUndo).toHaveBeenCalledWith("compare", "ocr");
|
||||
});
|
||||
|
||||
it("propagates failures so callers can surface an error toast", async () => {
|
||||
mockDismiss.mockRejectedValue(new Error("boom"));
|
||||
|
||||
const { result } = renderHook(() => useDismissToolRecommendation(), {
|
||||
wrapper: TestQueryProvider,
|
||||
});
|
||||
|
||||
await expect(result.current(null, "ocr")).rejects.toThrow("boom");
|
||||
});
|
||||
|
||||
it("a failing undo rejects so the caller can report it", async () => {
|
||||
mockDismiss.mockResolvedValue(undefined);
|
||||
mockUndo.mockRejectedValue(new Error("offline"));
|
||||
|
||||
const { result } = renderHook(() => useDismissToolRecommendation(), {
|
||||
wrapper: TestQueryProvider,
|
||||
});
|
||||
|
||||
const undo = await result.current("compare", "ocr");
|
||||
await expect(undo()).rejects.toThrow("offline");
|
||||
});
|
||||
|
||||
it("optimistically hides the tool only in the context it was dismissed from", async () => {
|
||||
mockFetch.mockImplementation(async (context) =>
|
||||
context === "compare"
|
||||
? [
|
||||
{ toolKey: "ocr", score: 5 },
|
||||
{ toolKey: "merge", score: 3 },
|
||||
]
|
||||
: [{ toolKey: "ocr", score: 4 }],
|
||||
);
|
||||
mockDismiss.mockResolvedValue(undefined);
|
||||
|
||||
const client = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false } },
|
||||
});
|
||||
const wrapper = ({ children }: { children: React.ReactNode }) => (
|
||||
<QueryClientProvider client={client}>{children}</QueryClientProvider>
|
||||
);
|
||||
|
||||
// Populate both a "compare" context list and the no-context list.
|
||||
await client.fetchQuery({
|
||||
queryKey: qk.toolRecommendations("compare", 8),
|
||||
queryFn: () => fetchToolRecommendations("compare", 8),
|
||||
});
|
||||
await client.fetchQuery({
|
||||
queryKey: qk.toolRecommendations("*", 8),
|
||||
queryFn: () => fetchToolRecommendations(null, 8),
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useDismissToolRecommendation(), {
|
||||
wrapper,
|
||||
});
|
||||
await result.current("compare", "ocr");
|
||||
|
||||
expect(client.getQueryData(qk.toolRecommendations("compare", 8))).toEqual([
|
||||
{ toolKey: "merge", score: 3 },
|
||||
]);
|
||||
// The other context keeps its entry: this dismissal does not apply there.
|
||||
expect(client.getQueryData(qk.toolRecommendations("*", 8))).toEqual([
|
||||
{ toolKey: "ocr", score: 4 },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,13 +1,8 @@
|
||||
import { useCallback, useMemo, useSyncExternalStore } from "react";
|
||||
import { useMemo, useSyncExternalStore } from "react";
|
||||
|
||||
import { useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import { useQuery } from "@tanstack/react-query";
|
||||
|
||||
import {
|
||||
ANY_CONTEXT,
|
||||
dismissToolRecommendation,
|
||||
fetchToolRecommendations,
|
||||
undoDismissToolRecommendation,
|
||||
} from "@app/api/toolRecommendations";
|
||||
import { fetchToolRecommendations } from "@app/api/toolRecommendations";
|
||||
import { qk } from "@app/query/keys";
|
||||
import {
|
||||
getLastCompletedTool,
|
||||
@@ -16,6 +11,9 @@ import {
|
||||
import { isValidToolId, ToolId } from "@app/types/toolId";
|
||||
|
||||
const RECOMMENDATIONS_STALE_TIME = 2 * 60 * 1000;
|
||||
|
||||
/** Query-key stand-in for "no tool has finished yet". */
|
||||
const NO_CONTEXT = "*";
|
||||
export const DEFAULT_RECOMMENDATION_LIMIT = 8;
|
||||
|
||||
/** The tool the user most recently completed; recommendations answer "what next after it". */
|
||||
@@ -24,8 +22,9 @@ export function useRecommendationContextTool(): ToolId | null {
|
||||
}
|
||||
|
||||
/**
|
||||
* Usage-ranked tool ids for the recommended section, or null when the backend
|
||||
* has no data (or no recommendations API) and the static list should be shown.
|
||||
* Usage-ranked tool ids for the "what next" suggestions shown after a tool
|
||||
* finishes, or null when the backend has no data (or no recommendations API)
|
||||
* and the curated list should be shown instead.
|
||||
*/
|
||||
export function useToolRecommendations(
|
||||
limit: number = DEFAULT_RECOMMENDATION_LIMIT,
|
||||
@@ -36,7 +35,7 @@ export function useToolRecommendations(
|
||||
const contextTool = useRecommendationContextTool();
|
||||
|
||||
const { data } = useQuery({
|
||||
queryKey: qk.toolRecommendations(contextTool ?? ANY_CONTEXT, limit),
|
||||
queryKey: qk.toolRecommendations(contextTool ?? NO_CONTEXT, limit),
|
||||
queryFn: () => fetchToolRecommendations(contextTool, limit),
|
||||
staleTime: RECOMMENDATIONS_STALE_TIME,
|
||||
retry: false,
|
||||
@@ -50,46 +49,3 @@ export function useToolRecommendations(
|
||||
|
||||
return { recommendedToolIds, contextTool };
|
||||
}
|
||||
|
||||
/**
|
||||
* Dismisses a recommendation for the given context (or everywhere when the
|
||||
* context is null), with an optimistic cache update; returns an undo callback.
|
||||
*/
|
||||
export function useDismissToolRecommendation(): (
|
||||
contextTool: ToolId | null,
|
||||
dismissedTool: ToolId,
|
||||
) => Promise<() => Promise<void>> {
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
return useCallback(
|
||||
async (contextTool: ToolId | null, dismissedTool: ToolId) => {
|
||||
// Dismissals are context-scoped, so only that context's cached lists are
|
||||
// touched; the key prefix stops short of the limit to cover every variant.
|
||||
const contextKey = [
|
||||
"editor",
|
||||
"toolRecommendations",
|
||||
contextTool ?? ANY_CONTEXT,
|
||||
];
|
||||
const invalidate = () =>
|
||||
void queryClient.invalidateQueries({ queryKey: contextKey });
|
||||
|
||||
queryClient.setQueriesData<{ toolKey: string; score: number }[] | null>(
|
||||
{ queryKey: contextKey },
|
||||
(existing) =>
|
||||
existing
|
||||
? existing.filter((r) => r.toolKey !== dismissedTool)
|
||||
: existing,
|
||||
);
|
||||
try {
|
||||
await dismissToolRecommendation(contextTool, dismissedTool);
|
||||
} finally {
|
||||
invalidate();
|
||||
}
|
||||
return async () => {
|
||||
await undoDismissToolRecommendation(contextTool, dismissedTool);
|
||||
invalidate();
|
||||
};
|
||||
},
|
||||
[queryClient],
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,223 +0,0 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { renderHook } from "@testing-library/react";
|
||||
|
||||
import {
|
||||
SubcategoryId,
|
||||
ToolCategoryId,
|
||||
ToolRegistryEntry,
|
||||
} from "@app/data/toolsTaxonomy";
|
||||
import { useToolSections } from "@app/hooks/useToolSections";
|
||||
import { useToolRecommendations } from "@app/hooks/useToolRecommendations";
|
||||
import { ToolId } from "@app/types/toolId";
|
||||
|
||||
// useToolSections imports the limit from here too, so the mock must supply it.
|
||||
const LIMIT = 8;
|
||||
vi.mock("@app/hooks/useToolRecommendations", () => ({
|
||||
DEFAULT_RECOMMENDATION_LIMIT: 8,
|
||||
useToolRecommendations: vi.fn(),
|
||||
}));
|
||||
|
||||
const mockUseToolRecommendations = vi.mocked(useToolRecommendations);
|
||||
|
||||
function makeTool(
|
||||
overrides: Partial<ToolRegistryEntry> = {},
|
||||
): ToolRegistryEntry {
|
||||
return {
|
||||
icon: null,
|
||||
name: "Tool",
|
||||
component: (() => null) as never,
|
||||
description: "",
|
||||
categoryId: ToolCategoryId.STANDARD_TOOLS,
|
||||
subcategoryId: SubcategoryId.GENERAL,
|
||||
automationSettings: null,
|
||||
...overrides,
|
||||
} as ToolRegistryEntry;
|
||||
}
|
||||
|
||||
function entry(id: string, tool: ToolRegistryEntry) {
|
||||
return { item: [id as ToolId, tool] as [ToolId, ToolRegistryEntry] };
|
||||
}
|
||||
|
||||
const registryFixture = [
|
||||
entry(
|
||||
"merge",
|
||||
makeTool({
|
||||
name: "Merge",
|
||||
categoryId: ToolCategoryId.RECOMMENDED_TOOLS,
|
||||
subcategoryId: SubcategoryId.GENERAL,
|
||||
}),
|
||||
),
|
||||
entry(
|
||||
"compare",
|
||||
makeTool({
|
||||
name: "Compare",
|
||||
categoryId: ToolCategoryId.RECOMMENDED_TOOLS,
|
||||
subcategoryId: SubcategoryId.DOCUMENT_REVIEW,
|
||||
}),
|
||||
),
|
||||
entry(
|
||||
"ocr",
|
||||
makeTool({ name: "OCR", subcategoryId: SubcategoryId.EXTRACTION }),
|
||||
),
|
||||
entry(
|
||||
"split",
|
||||
makeTool({ name: "Split", subcategoryId: SubcategoryId.PAGE_FORMATTING }),
|
||||
),
|
||||
entry(
|
||||
"removePassword",
|
||||
makeTool({
|
||||
name: "Remove password",
|
||||
subcategoryId: SubcategoryId.DOCUMENT_SECURITY,
|
||||
// Not ready: no component and no link, so never shown in Quick Access.
|
||||
component: null,
|
||||
}),
|
||||
),
|
||||
];
|
||||
|
||||
type SectionsResult = {
|
||||
sections: { key: string; subcategories: { tools: { id: ToolId }[] }[] }[];
|
||||
rankedRecommendationIds: Set<ToolId>;
|
||||
};
|
||||
|
||||
function sectionIds(result: SectionsResult, key: string): ToolId[] {
|
||||
const section = result.sections.find((s) => s.key === key);
|
||||
return section
|
||||
? section.subcategories.flatMap((sc) => sc.tools.map((t) => t.id))
|
||||
: [];
|
||||
}
|
||||
|
||||
const quickIds = (result: SectionsResult) => sectionIds(result, "quick");
|
||||
const allIds = (result: SectionsResult) => sectionIds(result, "all");
|
||||
const rankedIds = (result: SectionsResult) => [
|
||||
...result.rankedRecommendationIds,
|
||||
];
|
||||
|
||||
describe("useToolSections recommendations", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("keeps the static recommended list when no usage data exists", () => {
|
||||
mockUseToolRecommendations.mockReturnValue({
|
||||
recommendedToolIds: null,
|
||||
contextTool: null,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useToolSections(registryFixture));
|
||||
|
||||
expect(quickIds(result.current)).toEqual(["compare", "merge"]);
|
||||
expect(rankedIds(result.current)).toEqual([]);
|
||||
});
|
||||
|
||||
it("leads with the usage ranking in score order, then tops up from the static list", () => {
|
||||
mockUseToolRecommendations.mockReturnValue({
|
||||
recommendedToolIds: ["split", "ocr", "merge"] as ToolId[],
|
||||
contextTool: null,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useToolSections(registryFixture));
|
||||
|
||||
// 'compare' is the only curated entry the ranking did not already cover.
|
||||
expect(quickIds(result.current)).toEqual([
|
||||
"split",
|
||||
"ocr",
|
||||
"merge",
|
||||
"compare",
|
||||
]);
|
||||
expect(rankedIds(result.current)).toEqual(["split", "ocr", "merge"]);
|
||||
});
|
||||
|
||||
it("drops recommended ids that are unknown or not ready", () => {
|
||||
mockUseToolRecommendations.mockReturnValue({
|
||||
recommendedToolIds: ["removePassword", "ocr", "automate"] as ToolId[],
|
||||
contextTool: null,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useToolSections(registryFixture));
|
||||
|
||||
expect(quickIds(result.current)).toEqual(["ocr", "compare", "merge"]);
|
||||
expect(rankedIds(result.current)).toEqual(["ocr"]);
|
||||
});
|
||||
|
||||
it("falls back to the static list when no recommended id survives filtering", () => {
|
||||
mockUseToolRecommendations.mockReturnValue({
|
||||
recommendedToolIds: ["automate"] as ToolId[],
|
||||
contextTool: null,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useToolSections(registryFixture));
|
||||
|
||||
expect(quickIds(result.current)).toEqual(["compare", "merge"]);
|
||||
expect(rankedIds(result.current)).toEqual([]);
|
||||
});
|
||||
|
||||
it("never lists a tool in both Quick Access and All Tools", () => {
|
||||
mockUseToolRecommendations.mockReturnValue({
|
||||
recommendedToolIds: ["ocr", "merge"] as ToolId[],
|
||||
contextTool: null,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useToolSections(registryFixture));
|
||||
|
||||
const quick = quickIds(result.current);
|
||||
const all = allIds(result.current);
|
||||
expect(quick).toEqual(["ocr", "merge", "compare"]);
|
||||
expect(all.filter((id) => quick.includes(id))).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps the statically recommended tools in Quick Access when the ranking omits them", () => {
|
||||
// The regression this guards: a couple of tool runs used to replace the whole
|
||||
// curated list, collapsing Quick Access to one or two entries on a fresh install.
|
||||
mockUseToolRecommendations.mockReturnValue({
|
||||
recommendedToolIds: ["ocr"] as ToolId[],
|
||||
contextTool: null,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useToolSections(registryFixture));
|
||||
|
||||
expect(quickIds(result.current)).toEqual(["ocr", "compare", "merge"]);
|
||||
expect(allIds(result.current)).not.toContain("merge");
|
||||
});
|
||||
|
||||
it("tops the quick list up to the limit and no further", () => {
|
||||
const curated = Array.from({ length: LIMIT }, (_, i) =>
|
||||
entry(
|
||||
`static${i}`,
|
||||
makeTool({
|
||||
name: `Static ${i}`,
|
||||
categoryId: ToolCategoryId.RECOMMENDED_TOOLS,
|
||||
subcategoryId: SubcategoryId.GENERAL,
|
||||
}),
|
||||
),
|
||||
);
|
||||
mockUseToolRecommendations.mockReturnValue({
|
||||
recommendedToolIds: ["ocr"] as ToolId[],
|
||||
contextTool: null,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() =>
|
||||
useToolSections([...curated, entry("ocr", makeTool({ name: "OCR" }))]),
|
||||
);
|
||||
|
||||
// One ranked tool leads; the curated entries fill the remaining slots.
|
||||
const quick = quickIds(result.current);
|
||||
expect(quick).toHaveLength(LIMIT);
|
||||
expect(quick[0]).toBe("ocr");
|
||||
expect(quick).not.toContain(`static${LIMIT - 1}`);
|
||||
});
|
||||
|
||||
it("still hides the static recommended tools from All Tools when no ranking exists", () => {
|
||||
mockUseToolRecommendations.mockReturnValue({
|
||||
recommendedToolIds: null,
|
||||
contextTool: null,
|
||||
});
|
||||
|
||||
const { result } = renderHook(() => useToolSections(registryFixture));
|
||||
|
||||
expect(quickIds(result.current)).toEqual(["compare", "merge"]);
|
||||
expect(allIds(result.current)).not.toContain("merge");
|
||||
expect(allIds(result.current)).toEqual(
|
||||
expect.arrayContaining(["ocr", "split", "removePassword"]),
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -7,16 +7,8 @@ import {
|
||||
ToolRegistryEntry,
|
||||
} from "@app/data/toolsTaxonomy";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import {
|
||||
DEFAULT_RECOMMENDATION_LIMIT,
|
||||
useToolRecommendations,
|
||||
} from "@app/hooks/useToolRecommendations";
|
||||
import { ToolId } from "@app/types/toolId";
|
||||
|
||||
/** Tools that can actually open: have a component, an external link, or are navigational. */
|
||||
const isReadyTool = ({ tool, id }: { tool: ToolRegistryEntry; id: ToolId }) =>
|
||||
tool.component !== null || !!tool.link || id === "read" || id === "multiTool";
|
||||
|
||||
type SubcategoryIdMap = {
|
||||
[subcategoryId in SubcategoryId]: Array<{
|
||||
id: ToolId;
|
||||
@@ -52,7 +44,6 @@ export function useToolSections(
|
||||
searchQuery?: string,
|
||||
) {
|
||||
const { t } = useTranslation();
|
||||
const { recommendedToolIds } = useToolRecommendations();
|
||||
|
||||
const groupedTools = useMemo(() => {
|
||||
if (!filteredTools || !Array.isArray(filteredTools)) {
|
||||
@@ -71,12 +62,46 @@ export function useToolSections(
|
||||
return grouped;
|
||||
}, [filteredTools]);
|
||||
|
||||
const { sections, rankedRecommendationIds } = useMemo(() => {
|
||||
const sections: ToolSection[] = useMemo(() => {
|
||||
const getOrderIndex = (id: SubcategoryId) => {
|
||||
const idx = SUBCATEGORY_ORDER.indexOf(id);
|
||||
return idx === -1 ? Number.MAX_SAFE_INTEGER : idx;
|
||||
};
|
||||
|
||||
const quick = {} as SubcategoryIdMap;
|
||||
const all = {} as SubcategoryIdMap;
|
||||
|
||||
Object.entries(groupedTools).forEach(([c, subs]) => {
|
||||
const categoryId = c as ToolCategoryId;
|
||||
|
||||
Object.entries(subs).forEach(([s, tools]) => {
|
||||
const subcategoryId = s as SubcategoryId;
|
||||
// Build the 'all' collection without duplicating recommended tools
|
||||
// Recommended tools are shown in the Quick section only
|
||||
if (categoryId !== ToolCategoryId.RECOMMENDED_TOOLS) {
|
||||
if (!all[subcategoryId]) all[subcategoryId] = [];
|
||||
all[subcategoryId].push(...tools);
|
||||
}
|
||||
});
|
||||
|
||||
if (categoryId === ToolCategoryId.RECOMMENDED_TOOLS) {
|
||||
Object.entries(subs).forEach(([s, tools]) => {
|
||||
const subcategoryId = s as SubcategoryId;
|
||||
if (!quick[subcategoryId]) quick[subcategoryId] = [];
|
||||
// Only include ready tools (have a component or external link) in Quick Access
|
||||
// Special case: read and multiTool are navigational tools that don't need components
|
||||
const readyTools = tools.filter(
|
||||
({ tool, id }) =>
|
||||
tool.component !== null ||
|
||||
!!tool.link ||
|
||||
id === "read" ||
|
||||
id === "multiTool",
|
||||
);
|
||||
quick[subcategoryId].push(...readyTools);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
const sortSubs = (obj: SubcategoryIdMap) =>
|
||||
Object.entries(obj)
|
||||
.sort(([a], [b]) => {
|
||||
@@ -92,63 +117,6 @@ export function useToolSections(
|
||||
({ subcategoryId, tools }) as SubcategoryGroup,
|
||||
);
|
||||
|
||||
// Every tool starts in 'all'; whatever Quick Access ends up showing is removed
|
||||
// from it below, so a tool is never listed twice nor lost when the quick list changes.
|
||||
let quick = {} as SubcategoryIdMap;
|
||||
const all = {} as SubcategoryIdMap;
|
||||
|
||||
Object.entries(groupedTools).forEach(([c, subs]) => {
|
||||
const categoryId = c as ToolCategoryId;
|
||||
|
||||
Object.entries(subs).forEach(([s, tools]) => {
|
||||
const subcategoryId = s as SubcategoryId;
|
||||
if (!all[subcategoryId]) all[subcategoryId] = [];
|
||||
all[subcategoryId].push(...tools);
|
||||
|
||||
if (categoryId === ToolCategoryId.RECOMMENDED_TOOLS) {
|
||||
if (!quick[subcategoryId]) quick[subcategoryId] = [];
|
||||
// Only include ready tools (have a component or external link) in Quick Access
|
||||
// Special case: read and multiTool are navigational tools that don't need components
|
||||
quick[subcategoryId].push(...tools.filter(isReadyTool));
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// Ranked tools lead, curated ones top the list back up - a couple of runs may
|
||||
// reorder Quick Access but must never shrink it. One bucket keeps score order.
|
||||
const ranked = new Set<ToolId>();
|
||||
if (recommendedToolIds) {
|
||||
const byId = new Map<ToolId, ToolRegistryEntry>();
|
||||
filteredTools.forEach(({ item: [id, tool] }) => byId.set(id, tool));
|
||||
const dynamicTools = recommendedToolIds
|
||||
.filter((id) => byId.has(id))
|
||||
.map((id) => ({ id, tool: byId.get(id)! }))
|
||||
.filter(isReadyTool);
|
||||
if (dynamicTools.length > 0) {
|
||||
dynamicTools.forEach(({ id }) => ranked.add(id));
|
||||
const topUp = sortSubs(quick)
|
||||
.flatMap(({ tools }) => tools)
|
||||
.filter(({ id }) => !ranked.has(id));
|
||||
quick = {
|
||||
[SubcategoryId.GENERAL]: [...dynamicTools, ...topUp].slice(
|
||||
0,
|
||||
Math.max(dynamicTools.length, DEFAULT_RECOMMENDATION_LIMIT),
|
||||
),
|
||||
} as SubcategoryIdMap;
|
||||
}
|
||||
}
|
||||
|
||||
const quickIds = new Set(
|
||||
Object.values(quick).flatMap((tools) => tools.map(({ id }) => id)),
|
||||
);
|
||||
Object.keys(all).forEach((key) => {
|
||||
const subcategoryId = key as SubcategoryId;
|
||||
all[subcategoryId] = all[subcategoryId].filter(
|
||||
({ id }) => !quickIds.has(id),
|
||||
);
|
||||
if (all[subcategoryId].length === 0) delete all[subcategoryId];
|
||||
});
|
||||
|
||||
const built: ToolSection[] = [
|
||||
{
|
||||
key: "quick",
|
||||
@@ -162,13 +130,10 @@ export function useToolSections(
|
||||
},
|
||||
];
|
||||
|
||||
return {
|
||||
sections: built.filter((section) =>
|
||||
section.subcategories.some((sc) => sc.tools.length > 0),
|
||||
),
|
||||
rankedRecommendationIds: ranked,
|
||||
};
|
||||
}, [groupedTools, recommendedToolIds, filteredTools, t]);
|
||||
return built.filter((section) =>
|
||||
section.subcategories.some((sc) => sc.tools.length > 0),
|
||||
);
|
||||
}, [groupedTools]);
|
||||
|
||||
const searchGroups: SubcategoryGroup[] = useMemo(() => {
|
||||
if (!filteredTools || !Array.isArray(filteredTools)) {
|
||||
@@ -217,5 +182,5 @@ export function useToolSections(
|
||||
);
|
||||
}, [filteredTools, searchQuery]);
|
||||
|
||||
return { sections, searchGroups, rankedRecommendationIds };
|
||||
return { sections, searchGroups };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user