fix(license): apply the second review of #359
All checks were successful
PR Check — Frontend / frontend (pull_request) Successful in 1m43s
PR Check — Rust / rust (pull_request) Successful in 8m54s

- The idempotent-deactivation test now pins the route's other 404:
  license_not_found stays an error. The previous negative case was a
  403 that the deactivate route never returns, so swallowing
  license_not_found (an unknown key shown as deactivated, the local
  token deleted) went unnoticed.
- A test deactivates this machine and checks that the re-read activation
  status brings the Activate button back. Removing that re-read used to
  leave the suite green.
- The Activate button stays disabled while the list loads
  (selectCanActivate), the counterpart of selectCanDeactivate: a verify
  answer landing after a successful activation can no longer mark as
  fresh a list that misses this machine.
- CHANGELOG: the old /simpl-resultat address is redirected by the site
  since la-compagnie-maximus#153, so the "Buy" bullet now says the
  button opens the product page directly, in the app's language. The
  test comment is moved to the past tense.

Proven red on the fixed tree: idempotence swallowing license_not_found
1, no status re-read after deactivation 1, Activate live during a list
load 1.

Refs #270

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
le king fu 2026-09-27 08:09:01 -04:00
parent a051b96c87
commit b8e67c3d4f
7 changed files with 69 additions and 12 deletions

View file

@ -7,7 +7,7 @@
- L'activation, la désactivation et la liste des machines passent désormais par les points d'accès `/v1` du serveur de licences et nomment le produit, au lieu de passer par une redirection à chaque appel (#270). - L'activation, la désactivation et la liste des machines passent désormais par les points d'accès `/v1` du serveur de licences et nomment le produit, au lieu de passer par une redirection à chaque appel (#270).
- Un refus du serveur de licences est désormais expliqué dans votre langue : clé révoquée, expirée ou introuvable, nombre maximal de machines atteint, trop de tentatives (avec le délai à attendre), serveur injoignable. Il s'affichait auparavant en anglais sous un « Activation failed » générique, et une clé révoquée apparaissait comme « Aucune machine activée » (#270). - Un refus du serveur de licences est désormais expliqué dans votre langue : clé révoquée, expirée ou introuvable, nombre maximal de machines atteint, trop de tentatives (avec le délai à attendre), serveur injoignable. Il s'affichait auparavant en anglais sous un « Activation failed » générique, et une clé révoquée apparaissait comme « Aucune machine activée » (#270).
- Quand le nombre maximal de machines est atteint, la liste des machines s'ouvre d'elle-même pour en désactiver une, et le message reste affiché pendant son chargement. Désactiver une machine ne réinterroge plus le serveur, et désactiver une machine que le serveur considère déjà libérée compte désormais comme réussi (#270). - Quand le nombre maximal de machines est atteint, la liste des machines s'ouvre d'elle-même pour en désactiver une, et le message reste affiché pendant son chargement. Désactiver une machine ne réinterroge plus le serveur, et désactiver une machine que le serveur considère déjà libérée compte désormais comme réussi (#270).
- Le bouton « Acheter » de la carte Licence ouvrait une page qui n'existe pas ; il ouvre désormais la page produit dans la langue de l'application (#270). - Le bouton « Acheter » de la carte Licence ouvre désormais directement la page produit, dans la langue de l'application, au lieu d'une ancienne adresse qui ne fonctionne que grâce à une redirection du site (#270).
### Sécurité ### Sécurité

View file

@ -7,7 +7,7 @@
- Machine activation, deactivation and the machine list now call the license server's current `/v1` endpoints and name the product explicitly, instead of going through a redirect on every call (#270). - Machine activation, deactivation and the machine list now call the license server's current `/v1` endpoints and name the product explicitly, instead of going through a redirect on every call (#270).
- A refusal from the license server is now explained in your language: revoked, expired or unknown key, machine limit reached, too many attempts (with the wait time), server unreachable. It used to show a generic "Activation failed" in English, and a revoked key showed up as "No machines activated" (#270). - A refusal from the license server is now explained in your language: revoked, expired or unknown key, machine limit reached, too many attempts (with the wait time), server unreachable. It used to show a generic "Activation failed" in English, and a revoked key showed up as "No machines activated" (#270).
- When the machine limit is reached, the machine list opens by itself so you can deactivate one, and the message stays visible while it loads. Deactivating a machine no longer queries the server a second time, and deactivating a machine the server already considers free now counts as done (#270). - When the machine limit is reached, the machine list opens by itself so you can deactivate one, and the message stays visible while it loads. Deactivating a machine no longer queries the server a second time, and deactivating a machine the server already considers free now counts as done (#270).
- The "Buy" button of the License card opened a page that does not exist; it now opens the product page in the app's language (#270). - The "Buy" button of the License card now opens the product page directly, in the app's language, instead of an old address that only works through a redirect on the website (#270).
### Security ### Security

View file

@ -1099,17 +1099,19 @@ mod license_server_tests {
.await; .await;
assert_eq!(deactivate_idempotent(&server.url(), "SR-BASE-k", "machine-2").await, Ok(())); assert_eq!(deactivate_idempotent(&server.url(), "SR-BASE-k", "machine-2").await, Ok(()));
let mut revoked = mockito::Server::new_async().await; // The route's other 404: an unknown key must stay an error, never "deactivated"
let _r = revoked // (the command would also delete this machine's token). Only error.code decides.
let mut unknown = mockito::Server::new_async().await;
let _u = unknown
.mock("POST", "/v1/licenses/deactivate") .mock("POST", "/v1/licenses/deactivate")
.with_status(403) .with_status(404)
.with_header("content-type", "application/json") .with_header("content-type", "application/json")
.with_body(r#"{"error":{"code":"license_revoked","message":"License has been revoked"}}"#) .with_body(r#"{"error":{"code":"license_not_found","message":"License not found"}}"#)
.create_async() .create_async()
.await; .await;
assert_eq!( assert_eq!(
deactivate_idempotent(&revoked.url(), "SR-BASE-k", "machine-2").await, deactivate_idempotent(&unknown.url(), "SR-BASE-k", "machine-2").await,
Err(LicenseError::LicenseRevoked) Err(LicenseError::LicenseNotFound)
); );
} }

View file

@ -22,6 +22,7 @@ export default function LicenseCard() {
toggleList, toggleList,
showEmptyState, showEmptyState,
canDeactivate, canDeactivate,
canActivate,
} = useLicenseMachines(hasLicense); } = useLicenseMachines(hasLicense);
const handleSubmit = async (e: React.FormEvent) => { const handleSubmit = async (e: React.FormEvent) => {
@ -156,7 +157,7 @@ export default function LicenseCard() {
<button <button
type="button" type="button"
onClick={() => void activate()} onClick={() => void activate()}
disabled={machines.activating} disabled={!canActivate}
className="flex items-center gap-1 px-3 py-1 bg-[var(--primary)] text-white rounded-lg hover:opacity-90 transition-opacity text-xs disabled:opacity-50" className="flex items-center gap-1 px-3 py-1 bg-[var(--primary)] text-white rounded-lg hover:opacity-90 transition-opacity text-xs disabled:opacity-50"
> >
{machines.activating && <Loader2 size={12} className="animate-spin" />} {machines.activating && <Loader2 size={12} className="animate-spin" />}

View file

@ -4,16 +4,19 @@ import {
initialLicenseMachinesState, initialLicenseMachinesState,
selectShowEmptyState, selectShowEmptyState,
selectCanDeactivate, selectCanDeactivate,
selectCanActivate,
shouldLoadOnOpen, shouldLoadOnOpen,
performActivate, performActivate,
performDeactivate, performDeactivate,
loadMachines, loadMachines,
loadActivationStatus,
type LicenseMachinesAction, type LicenseMachinesAction,
type LicenseMachinesDeps, type LicenseMachinesDeps,
type LicenseMachinesState, type LicenseMachinesState,
} from "./useLicenseMachines"; } from "./useLicenseMachines";
import { import {
parseLicenseError, parseLicenseError,
type ActivationStatus,
type LicenseError, type LicenseError,
type LicenseErrorCode, type LicenseErrorCode,
type LicenseResult, type LicenseResult,
@ -218,6 +221,23 @@ describe("selectCanDeactivate", () => {
}); });
}); });
describe("selectCanActivate", () => {
it("allows an activation when nothing is running", () => {
expect(selectCanActivate(initialLicenseMachinesState)).toBe(true);
});
it("blocks the button while an activation is running", () => {
expect(selectCanActivate(reduce([{ type: "ACTIVATE_START" }]))).toBe(false);
});
it("blocks the button while the list loads", () => {
// Otherwise a verify answer landing after a successful activation would
// mark as fresh a list that no longer includes this machine.
const loading = reduce([{ type: "TOGGLE_LIST" }, { type: "LIST_LOAD_START" }]);
expect(selectCanActivate(loading)).toBe(false);
});
});
describe("shouldLoadOnOpen", () => { describe("shouldLoadOnOpen", () => {
const loaded = reduce([{ type: "LIST_LOAD_START" }, { type: "LIST_LOAD_DONE", machines: [] }]); const loaded = reduce([{ type: "LIST_LOAD_START" }, { type: "LIST_LOAD_DONE", machines: [] }]);
@ -349,6 +369,29 @@ describe("performDeactivate", () => {
expect(deps.refresh).toHaveBeenCalledTimes(1); expect(deps.refresh).toHaveBeenCalledTimes(1);
}); });
it("re-reads the local status, so deactivating this machine brings back Activate", async () => {
const getActivationStatus = vi
.fn<() => Promise<ActivationStatus>>()
.mockResolvedValueOnce({ is_activated: true, machine_id: "this" })
.mockResolvedValueOnce({ is_activated: false, machine_id: "this" });
const { deps, getState } = harness({ getActivationStatus });
await loadActivationStatus(deps);
await loadMachines({
...deps,
listActivatedMachines: vi.fn(async () => ({
ok: true as const,
value: [machine("this"), machine("b")],
})),
});
expect(getState().activation?.is_activated).toBe(true);
// Rust deletes this machine's activation token on success; only the
// re-read status turns the card back from "Activated" to "Activate".
await performDeactivate(deps, "this");
expect(getState().activation?.is_activated).toBe(false);
expect(getActivationStatus).toHaveBeenCalledTimes(2);
});
it("shows a refusal on the list and releases the busy state", async () => { it("shows a refusal on the list and releases the busy state", async () => {
const { deps, getState } = harness({ const { deps, getState } = harness({
deactivateMachine: vi.fn(async () => ({ deactivateMachine: vi.fn(async () => ({

View file

@ -135,6 +135,15 @@ export function selectCanDeactivate(state: LicenseMachinesState): boolean {
return state.deactivatingId === null && !state.listLoading; return state.deactivatingId === null && !state.listLoading;
} }
/**
* The Activate button is live only when no activation and no list reload is running.
* Same reason as for deactivation: a verify answer landing after a successful
* activation would mark as fresh a list that no longer includes this machine.
*/
export function selectCanActivate(state: LicenseMachinesState): boolean {
return !state.activating && !state.listLoading;
}
/** Whether opening the list (from `state`, before the toggle) should load it. */ /** Whether opening the list (from `state`, before the toggle) should load it. */
export function shouldLoadOnOpen(state: LicenseMachinesState): boolean { export function shouldLoadOnOpen(state: LicenseMachinesState): boolean {
return !state.listOpen && !state.listLoading && (!state.listLoaded || state.listError !== null); return !state.listOpen && !state.listLoading && (!state.listLoaded || state.listError !== null);
@ -239,5 +248,6 @@ export function useLicenseMachines(enabled: boolean) {
toggleList, toggleList,
showEmptyState: selectShowEmptyState(state), showEmptyState: selectShowEmptyState(state),
canDeactivate: selectCanDeactivate(state), canDeactivate: selectCanDeactivate(state),
canActivate: selectCanActivate(state),
}; };
} }

View file

@ -1,9 +1,10 @@
import { describe, it, expect } from "vitest"; import { describe, it, expect } from "vitest";
import { purchaseUrl } from "./purchaseUrl"; import { purchaseUrl } from "./purchaseUrl";
// The old constant pointed at /simpl-resultat, a path the site never served // The old constant pointed at /simpl-resultat, a path the site did not serve
// (307 to /fr/simpl-resultat, then 404). The product page lives under // when #270 was filed (307 to /fr/simpl-resultat, then 404). The site has
// /{locale}/produits/simpl-resultat (issue #270). // redirected it since la-compagnie-maximus#153; the app now links the
// localized product page, /{locale}/produits/simpl-resultat, directly.
describe("purchaseUrl", () => { describe("purchaseUrl", () => {
it("opens the French product page when the app is in French", () => { it("opens the French product page when the app is in French", () => {
expect(purchaseUrl("fr")).toBe( expect(purchaseUrl("fr")).toBe(