Skip to content

feat(webhooks): native PoracleNG v2 tracking rules, v1 via adapter - #1273

Open
Fabio1988 wants to merge 8 commits into
WatWowMap:mainfrom
Fabio1988:feat/poracle-v2-api
Open

Fabio1988 wants to merge 8 commits into
WatWowMap:mainfrom
Fabio1988:feat/poracle-v2-api

Conversation

@Fabio1988

@Fabio1988 Fabio1988 commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

PoracleNG's v1 tracking, humans and profiles routes are frozen and deprecated, and new behaviour only ships on /api/v2. This PR makes ReactMap's Poracle integration v2-native: from the Poracle service through GraphQL, the webhook store and the editor, rules are PoracleNG v2 rules. v1 is kept only as an adapter at the edge, for PoracleJS and older PoracleNG.

The first version of this PR converted v2 responses back into v1 rows internally (jfberry's review: "why convert it internally?"). That is gone; this is a rewrite of that approach, squashed into one commit.

Requirements

v2 needs PoracleNG develop at jfberry/PoracleNG@715c0d7e (merge of jfberry/PoracleNG#328) or later, until that reaches main. Earlier v2 versions use a different v2 shape (lat/lon, no invasion grunt_type, required pokemon_id), and this PR does not support it. Servers without a v2 API (PoracleJS) keep working through the v1 adapter.

How it works

  • Detection is unchanged. Webhook option apiVersion (auto by default, or v1 / v2). auto fetches the public /openapi.json once and uses v2 only if every v2 route ReactMap uses is listed. PoracleJS has no spec, so it stays on v1 quietly.
  • One rule shape everywhere: v2. null means any, enums are names (team: "valor", gender: "female", rsvp_changes: "rsvp_only"), and clean, edit and summary are separate booleans. GraphQL returns rule lists as JSON, so ReactMap no longer drops fields it has no schema for. Settings ReactMap has no input for (costume, overrides, PVP evolution, battle_changes, …) are therefore kept on edit without any merging logic.
  • server/src/services/poracle/:
    • http.js: one HTTP client. Error messages come from problem+json detail and errors[] (v2) or message (v1). Network failures become "Poracle server unreachable" / "timed out" and don't expose the internal Poracle URL to the browser.
    • trackingV2.js: new rules in one batch POST; edits are PUT …/tracking/{type}/{uid}, so the rule keeps its uid; bulk delete via ?uid=.
    • trackingV1.js + v1Adapter.js: same interface over the v1 routes. Converts v2 rules to v1 rows (sentinels, packed clean bitmask) and back. Every rule is converted before anything is sent, so a rule v1 can't represent is reported as an error and nothing is half-written.
  • New incident category (gold stop, Kecleon, showcase), its own tab, as in PoracleNG v2. On v1 servers incident rules map to the invasion rows they used to be.
  • v2-only features on v1 servers: the edit and summary switches are shown only when the server supports them (capabilities in the webhook context). An incident type v1 has no name for is rejected with a readable error rather than dropped silently.
  • Editor (WebhookAdv.jsx) builds its form from the rule (ruleForm.js: ruleToForm / formToRule). "Any" selects map to null, and the template select has a "Default" entry (null, server default).
  • Errors are shown. Every tracking mutation returns {status, message}; the manage, selecting and tracked-tile views and the gym popup's quick gym show the error instead of a success toast.
  • Humans and profiles keep using the v2 action endpoints from the first version (stepped active hours are kept).

Test plan

  • server/test/poracleTracking.test.js: v1 adapter round trips for every category, v2 and v1 clients against a stub server (batch POST, PUT by uid, bulk delete, invasion/incident split on v1, error message extraction, transport errors)
  • server/test/ruleForm.test.js: rule ⇄ form conversion, tile ids, null handling
  • server/test/webhookQueries.test.js: client GraphQL documents validate against the server schema
  • Full server suite with node --test (local Node 22.23.3 is one patch below the engine pin, so yarn test refuses to start): 203/203 pass. eslint clean, vite build ok
  • Rules built from every category's defaults validated with ajv against PoracleNG's v2 OpenAPI rule schemas. The only mismatch is null on the nullable enums (gender, team, rsvp_changes), which PoracleNG accepts (its validator skips null there, and TestV2_EveryNullableFieldRoundTripsNull sends it)
  • Manual check against a live PoracleNG: list, add, edit and delete per category (incl. incident), editing a rule created by the bot, quick gym, profiles
  • Manual check against PoracleJS: detected as v1, rules round-trip through the adapter

Known limitations

  • ReactMap still can't set costume, overrides or PVP evolution; it keeps them on edit.
  • Only en has the new locale keys (incident, edit, summary, default_template); other languages fall back.

🤖 Generated with Claude Code

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-29T18:41:48.149579Z fa12aca Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8afb8d4091

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/src/services/Poracle.js Resolved
@Fabio1988

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa12aca02c

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/src/services/poracleV2.js Resolved
@jfberry

jfberry commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Isn't this half baked? If you are supporting v2 why convert it internally? Feels like an agent just did half the job

@Fabio1988
Fabio1988 force-pushed the feat/poracle-v2-api branch from fa12aca to 003b3e8 Compare October 30, 2026 08:59
@Fabio1988 Fabio1988 changed the title feat(webhooks): support the PoracleNG v2 API alongside v1 feat(webhooks): native PoracleNG v2 tracking rules, v1 via adapter Oct 30, 2026
@Fabio1988

Copy link
Copy Markdown
Contributor Author

@jfberry Fair point. The first version translated v2 back into v1 rows internally, which threw away what v2 adds. I've reworked it (force-pushed, one commit): rules are now v2-shaped end to end, from the Poracle service through GraphQL (JSON rule lists), the store and the editor. That means null = any, named enums, separate clean/edit/summary, PUT by uid, and incident as its own category. v1 is only an adapter at the edge for PoracleJS and older PoracleNG. Details are in the updated description.

@Mygod

Mygod commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator

Several v2 schema mismatches break rule creation, invasion editing, and location updates; v1 list errors also regress. Changed-file lint/format checks and all 15 form/query tests passed. HTTP-backed tests were blocked by sandbox loopback restrictions; production build, standalone typechecking, and browser verification were not performed in this focused review.

Full review comments:

  • [P1] Strip the tile's all flag before submitting rules — src/features/webhooks/services/ruleForm.js:158-162
    Clicking a selection tile adds all: false in SelectorItem.handleClick, but FORM_ONLY_FIELDS does not include all. It consequently survives formToRule and reaches PoracleNG v2, whose strict schema rejects unknown properties. Ordinary click-to-select additions therefore fail with HTTP 422. Remove this editor-only field before submission.

  • [P1] Preserve native v2 invasion targeting fields — server/src/services/poracle/trackingV2.js:14-18
    PoracleNG's v2InvasionRule requires exactly one of type_id, grunt_id, everything, or boss; it does not accept grunt_type. New invasion forms still submit grunt_type, while this branch removes the valid targeting fields from existing rules before PUT. Both creation and editing therefore fail with HTTP 422. Use native targeting throughout the v2 path and translate legacy grunt_type inside the v1 adapter.

  • [P2] Send location coordinates using the v2 field names — server/src/services/Poracle.js:820-825
    When v2 is selected, every location update sends latitude and longitude, but PoracleNG's V2LocationBody requires lat and lon. The strict endpoint rejects this body with HTTP 422. Because fetchJson absorbs that failure and this method subsequently fetches the human, the UI receives the unchanged location without the validation error.

  • [P2] Keep the catch-all Pokémon ID as zero — server/src/services/Poracle.js:921-921
    The 0-0 catch-all filter is generated from these defaults without overriding pokemon_id. Changing it to null makes catch-all submissions invalid on v2: unlike optional filters, V2PokemonRule.pokemon_id is a required integer and uses explicit 0 for all Pokémon. Preserve that value so the catch-all tile and non-individual global tracking remain usable.

  • [P2] Preserve application-level errors from v1 list responses — server/src/services/poracle/trackingV1.js:23-25
    PoracleJS returns some failures as HTTP 200, including { status: 'error', message: 'User not found' } from invasion lists when the remote account no longer exists. This fallback converts that response into an empty list, and #runTracking reports success. A mocked comparison confirmed that the base preserved the error while this patch returns { status: 'ok', invasion: [] }. Check the v1 response status before extracting rows.

  • [P3] Add the incident search-label translation — packages/locales/lib/human/en.json:365-365
    The new category makes WebhookSearch request search_incidents, but that key is absent from the English locale. Opening the events tab therefore displays the raw key as its search label. Add the English entry alongside the category name, as required by AGENTS.md:13.

@Fabio1988

Copy link
Copy Markdown
Contributor Author

@Mygod Thanks for the review. Three findings are fixed in 0cb4121:

  • [P1] all flag: added to FORM_ONLY_FIELDS, so the tile's flag never reaches the rule (test covers it).
  • [P2] v1 200 status: 'error': http.js treats a {status: 'error'} body as an error regardless of HTTP status, so "User not found" surfaces again (test added).
  • [P3] search_incidents: added to en.json.

The other three are differences between PoracleNG main and develop. The v2 API changed in ways that break older clients in jfberry/PoracleNG#303 and #328 (listed in docs/v2-api-design.md under "Changes"), and this PR targets the current v2 API:

  • invasion grunt_type: accepted as a targeting field on develop (v2InvasionRule.GruntType: "The stored targeting value, and the ONE targeting field every read emits").
  • location body: develop uses latitude / longitude everywhere ("Coordinates are latitude / longitude everywhere" in Floating Weather Icon #303); lat / lon is the old shape.
  • catch-all pokemon_id: nullable on develop ("Omit (or null) for the catch-all that matches EVERY pokemon — stored as 0").

v2 support therefore requires PoracleNG develop at 715c0d7e (merge of #328) or later, until it lands in main. I've added that to the description. There is no fallback for the older v2 shape; servers without v2 use the v1 adapter.

@Fabio1988
Fabio1988 force-pushed the feat/poracle-v2-api branch 2 times, most recently from 17f4ffb to 33c588b Compare November 2, 2026 17:47
@Mygod

Mygod commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

The patch contains reproducible v2 payload incompatibilities and a filtered-list synchronization regression. Focused form/query tests and lint/formatting passed; HTTP-stub tests were blocked by loopback EPERM, and production build/live UI validation was not run.

Full review comments:

  • [P1] Preserve the required v2 invasion selector — server/src/services/poracle/trackingV2.js:14-18
    PoracleNG v2 requires exactly one of type_id, grunt_id, everything, or boss, and rejects grunt_type. This removes the required selector from edits, while useGenFilters still generates grunt_type for new rules, so both creation and editing fail with HTTP 422. Preserve v2 targeting throughout the editor and translate to grunt_type only in the v1 adapter.

  • [P2] Keep mutation results synchronized with the filtered query — src/store/useWebhookStore.js:219-221
    With a tracked-search filter active, mutations return the complete category list, but this replaces the filtered store that TrackedTile indexes. Meanwhile, Tracked still uses the filtered Apollo query, which JSON-scalar mutations no longer update through normalization. Editing the sole Pikachu result can therefore display an unrelated first rule, and a subsequent deletion targets that rule's UID. Update/refetch the query and derive the filtered view instead of publishing the unfiltered response directly.

  • [P2] Initialize catch-all Pokémon rules with an integer ID — src/features/webhooks/hooks/useGenFilters.js:44-45
    Selecting the catch-all Pokémon tile now produces pokemon_id: null from the defaults, and formToRule sends that unchanged. Unlike optional wildcard fields, PoracleNG v2 requires an integer pokemon_id; its catch-all value is 0. Consequently, saving this tile fails with HTTP 422 on v2, although the v1 adapter masks the problem by converting null to zero. Initialize this tile explicitly with pokemon_id: 0.

  • [P2] Translate Sunday between the UI and v2 weekday conventions — server/src/services/Poracle.js:36-38
    EditView and the existing translations represent Sunday as 7, but PoracleNG v2 accepts weekdays 0–6, with Sunday represented by 0. Copying the day unchanged makes Sunday schedule saves fail validation. Conversely, fromV2ActiveHours leaves Sunday as zero, producing the missing day_0 label. Map Sunday in both directions at this adapter boundary.

  • [P3] Use declared translation keys without fallback strings — src/features/webhooks/WebhookAdv.jsx:521-525
    For an unfamiliar incident type, this introduces an undeclared incident_<id> translation key and substitutes literal text. The new default-template label also supplies an English fallback. AGENTS.md:13 explicitly requires English entries and prohibits fallback strings. Use a declared interpolated label for unknown incidents and remove the new fallback arguments.

@jfberry

jfberry commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

@Fabio1988 I notice factual mistakes about the API in this review, so be careful just accepting it

@Fabio1988

Copy link
Copy Markdown
Contributor Author

@Mygod Checked each point against PoracleNG develop (the API version this PR targets, see Requirements in the description). Fixed in 782e4e7:

  • [P2] filtered list sync: confirmed. Mutation results now update the tracked query's cache instead of the store, so useGetWebhookData applies the active search and the list rows and tiles index the same rules. Manage uses the same handling.
  • [P3] translation fallbacks: removed the 'Default' fallback, and unknown incident types use a declared incident_unknown label (Pokestop Event #{{id}}) instead of an undeclared key.

Not changed, because they don't match the current v2 API:

  • [P1] invasion selector: develop accepts grunt_type as a targeting field (v2InvasionRule.GruntType), as answered above.
  • [P2] catch-all pokemon_id: nullable on develop; null is the catch-all, stored as 0.
  • [P2] Sunday: develop uses ISO weekdays, day 1–7 with 7 = Sunday (v2ActiveHourEntry: "ISO day of week (1=Monday … 7=Sunday)"), which is what ReactMap's UI already uses. 0–6 was the older v2 shape.

The fixes from the first round are now in 33c588b after the rebase onto main (0cb4121 no longer exists).

@Mygod

Mygod commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Yes, the previous review was looking at outdated PoracleNG.

The patch introduces rule-editing regressions and incomplete incident/error handling. Focused form and GraphQL-document tests plus changed-file lint/format checks passed; HTTP-backed tests were blocked by the sandbox's localhost bind restriction (listen EPERM).

Full review comments:

  • [P2] Preserve existing wildcard PvP rank ceilings — src/features/webhooks/services/ruleForm.js:93-95
    When pvpFilterMaxRank is below 4096, loading a PvP rule with pvp_ranking_worst: null replaces its unlimited ceiling with that configured maximum. formToRule() then saves the concrete value because this slider has anyHigh: false. Consequently, changing only the template can silently narrow an existing rule to the top 100 or top 10. Preserve the original wildcard when the rank constraint has not changed.

  • [P2] Reconcile location overrides when changing distance mode — src/features/webhooks/services/ruleForm.js:143-145
    For an existing rule with override_location_label, disabling “By Distance” sends distance: 0 while retaining the label, which Poracle rejects. Conversely, enabling distance on a rule with override_areas retains those areas and produces another mutually exclusive combination. Neither hidden field can be cleared through this editor. Clear conflicting overrides when the user switches modes, while preserving them for unrelated edits.

  • [P2] Honor the All Forms choice in global edits — src/features/webhooks/WebhookAdv.jsx:842-846
    Using “Apply to all” with “All Forms” enabled now leaves selected non-default forms unchanged: GLOBAL_DERIVED removes allForms from the shared settings, so each tile retains its previous allForms: false and submits its specific form. The global editor still exposes this control, but saving it has no effect. Preserve tile identities without discarding the user's explicit all-forms constraint.

  • [P2] Propagate tracking read failures to the client — server/src/services/Poracle.js:766-768
    When a tracking GET fails, this now returns an empty list with status: 'error', but ALL_PROFILES selects neither status nor message. GraphQL therefore delivers an apparently successful empty list, and useGetWebhookData() clears the displayed rules and shows “no alerts” rather than entering its error branch. Select and propagate the error fields for tracking reads, or keep read failures as GraphQL errors.

  • [P2] Map incident templates to invasion metadata on v1 — server/src/services/Poracle.js:1470-1470
    On PoracleJS/v1, event templates are listed under invasion, not incident. Moving these rules into this category makes WebhookAdv.getMenuItems() look up templates.incident, while #fetchTemplates() supplies no alias. Gold Stop, Kecleon, and Showcase rules consequently lose their non-default template choices. Adapt the legacy template metadata alongside the new category, consistent with the touch-point requirement in AGENTS.md:15.

  • [P2] Apply the invasion disable flag to incident availability — server/src/services/Poracle.js:1450-1451
    When Poracle reports disabledHooks: ['invasion'], this new category still passes getAllowedCategories() because that method compares keys literally. Gold Stop and Kecleon processing is also disabled by the upstream invasion flag, so users can now create subscriptions through an available-looking tab that cannot deliver alerts. Carry the underlying disable flag into incident availability, as required by the new-key plumbing guidance in AGENTS.md:15.

@Fabio1988

Copy link
Copy Markdown
Contributor Author

@Mygod All six addressed in d2af983:

  • [P2] wildcard PvP ceiling: confirmed. When the stored value is null and the slider still sits at the default it was shown with, the save keeps null. A rule that only now becomes a PvP entry still saves the ceiling the slider shows, since it had none to keep.
  • [P2] distance mode vs overrides: confirmed. Switching to areas clears override_location_label; switching to distance clears override_areas. Unrelated edits keep both.
  • [P2] All Forms in global edits: confirmed. A ticked All Forms now applies to every selected tile; unticked, each tile keeps its own form.
  • [P2] tracking read failures: confirmed. A failed read now returns no list (null) instead of [], and ALL_PROFILES selects status and message, so useGetWebhookData shows the error and keeps the last list. A fresh list from a later save resets the cached status.
  • [P2] incident templates on v1: confirmed for PoracleJS. On v1 incident uses the invasion template list. PoracleNG has its own incident template type, so v2 is unchanged.
  • [P2] invasion disable flag: disabledHooks is never populated today, but getAllowedCategories now hides incident when invasion is disabled, and on v1 also when invasion is in blocked_alerts. On v2 incident is its own alert type for blocked_alerts.

Tests added for each; 217/217 server tests, lint, prettier and build pass locally.

@Mygod

Mygod commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

The patch introduces a cross-user write vulnerability and reproducible form/search regressions. All 29 focused pure tests and changed-file lint/formatting checks passed; HTTP tests were blocked by localhost listener permissions, and production build/live integration validation was not performed.

Full review comments:

  • [P1] Validate rule UIDs before inserting them into v2 paths — server/src/services/poracle/trackingV2.js:53-58
    For v2 webhooks, the GraphQL data argument accepts arbitrary JSON, so uid is not necessarily numeric. Sending uid: '../../../victim/tracking/pokemon/7' makes node-fetch normalize this URL to /api/v2/humans/victim/tracking/pokemon/7, still authenticated with the server's Poracle secret. This bypasses the resolver-derived user ID and permits overwriting another user's rule. Reject non-integer UIDs before constructing the request path.

  • [P2] Preserve the global preset's explicit All Forms default — src/features/webhooks/hooks/useGenFilters.js:35-41
    Opening Apply to All now calls ruleToForm with no identity, which derives allForms: true from the default null form despite ui.defaults.allForms being false. Consequently, changing only an IV threshold or template applies All Forms to every selected tile, and formToRule replaces their specific form IDs with null. Initialize the global preset without deriving All Forms from an absent identity, so selected forms remain intact unless the user explicitly enables that option.

  • [P2] Synchronize filtered rows when retaining a failed read's data — src/features/webhooks/hooks/useGetWebhookData.js:49-54
    After a successful load followed by a failed poll, changing the search filters previousData here, but the error branch below never updates the category store. Tracked therefore receives the newly filtered array while TrackedTile and Select All use the stale array. For example, searching for the second rule produces a one-row list whose tile still displays and operates on the first rule. Keep both consumers on the same filtered snapshot even when displaying a read error.

  • [P3] Remove the fallback string from named enum labels — src/features/webhooks/WebhookAdv.jsx:113-113
    The second argument supplies raw API values such as male or rsvp_only as fallback strings, introducing this behavior for gender and RSVP labels. This violates the explicit no-fallback localization convention in AGENTS.md:13. Use the translation key alone and add any missing English entries instead.

@Fabio1988

Copy link
Copy Markdown
Contributor Author

@Mygod All four confirmed and fixed in bef2108:

  • [P1] uid in v2 paths: confirmed. The same applied to every client value that reached a URL, not only the rule uid, so all of them are now checked before any request is made:

    • rule uids (PUT path and the bulk ?uid= list) and profile numbers (v2 update/delete/copy, v1 switchProfile/copy/delete) must be digits;
    • v1 setLocation coordinates must be finite numbers;
    • an unknown category is refused instead of falling through to the tracking path. That fallthrough predates this PR (main had it too), but it took a client-controlled path segment as well;
    • v1 profile actions are limited to add/update/copy/byProfileNo.

    Refused input returns a 400-type error. A test sends ../../../victim/... through each route and asserts no write request reaches the server.

  • [P2] apply-to-all All Forms: confirmed, introduced by my previous fix. The preset is now built by globalPreset() in ruleForm.js: identity fields removed and All Forms taken from the defaults (off), so only an explicit tick applies it to every tile.

  • [P2] filtered rows during a failed read: confirmed. The store is now set from the same filtered rows whether or not the read failed, so Tracked, TrackedTile and Select All use one snapshot. The error alert is in its own effect and fires once per read result, not on every search change.

  • [P3] label fallback: confirmed. Removed it. The team/gender/rsvp keys come from the game locale, except genderless, so gender_3 is now in en.json.

219/219 server tests, lint, prettier and build pass locally.

@Mygod

Mygod commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

The v1 adapter introduces a rule-placement regression during profile switches. Focused form/query tests and changed-file lint/format checks passed; HTTP-backed tests were blocked by sandbox socket restrictions.

Review comment:

  • [P2] Preserve the originating profile on v1 Pokémon edits — server/src/services/poracle/v1Adapter.js:335-339
    If a schedule or bot command switches profiles while a Pokémon rule is being edited, the adapter drops the rule's original profile_no. Poracle's v1 Pokémon POST defaults the missing value to the currently active profile and updates by uid, so saving silently moves the existing rule between profiles. This is reproducible against the PoracleJS handler. Preserve the originating profile through the adapter's read/write round trip.

@Fabio1988

Copy link
Copy Markdown
Contributor Author

@Mygod Confirmed and fixed in c76fe1c.

Checked against both v1 servers: PoracleJS apiTrackingMonster.js (profile_no: +defaultTo(row.profile_no, +currentProfileNo), then updateQuery('monsters', row, { uid })) and PoracleNG's v1 trackingMonster.go (same default, uid rows applied as updates). The adapter dropped profile_no in fromV1Row/toV1Row, so an edit made across a profile switch moved the rule.

fromV1Row now keeps the row's profile_no, and toV1Row sends it back. The client builds the saved rule from the original, so an edit keeps the profile it was read from. New rules still have none and go to the active profile. Tests cover the adapter round trip and the form keeping the original profile.

The other v1 categories don't update by uid (PoracleJS diffs them within the current profile), so only pokemon could move. v2 isn't affected: PUT …/{uid} checks ownership within the active profile, so an edit across a switch is refused with a 404 instead of moving the rule.

221/221 server tests, lint and prettier pass locally.

@Mygod

Mygod commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

The new v2 profile path silently converts upstream HTTP failures into empty profile results. The production build, changed-file lint/format checks, and 32 focused tests passed; HTTP-backed tests were blocked by sandbox localhost-listening restrictions.

Review comment:

  • [P2] Reject failed responses in the v2 non-tracking client — server/src/services/Poracle.js:514-515
    If the v2 profiles endpoint returns HTTP 503, fetchJson returns the failed Response instead of rejecting. Consequently, #profileManagementV2 sees no profiles property and returns { profile: [] }, replacing the cached profile/schedule list with an apparently successful empty result. This was reproduced with a failed-response stub. Use the failure-propagating Poracle transport here so upstream failures cannot masquerade as empty data.

@Fabio1988

Copy link
Copy Markdown
Contributor Author

@Mygod Confirmed and fixed in 6a97dcd.

#sendV2 used fetchJson, which returns the failed Response on a non-2xx instead of throwing. #profileManagementV2 then read profiles off it, got undefined, and fell back to []. So a 503 on GET /api/v2/humans/{id}/profiles came back as an apparently valid empty list.

Changes:

  • #sendV2 now goes through the same Poracle transport as tracking (createPoracleHttp), which throws a PoracleRequestError on any HTTP or transport failure.
  • Profile actions now work like #runTracking. Write errors and read errors are kept separate. A failed read gives profile: null, not [], and the result has status: 'error' and the message. A failed write still returns the current list along with the error.
  • The v2 human actions (start/stop, switchProfile, setLocation, setAreas, oneHuman, humans) return { status: 'error', message } without a human. Before this, a failure either threw a TypeError on human.enabled or passed the Response through.
  • Client: profile and human mutation handlers (handleUpdate, and a new applyHumanResult in the store) only replace the profiles or human on screen when the result includes them, and show the error otherwise. NewProfile now goes through handleUpdate as well. HookSelection read data.webhook, but the webhookChange mutation returns data.webhookChange, so it never applied the human; it now reads the right field.

v1's non-tracking calls are unchanged. They still use the original #sendRequest/fetchJson, but they don't hide failures as empty data: #processProfiles(undefined) and #oneHuman throw instead.

New tests cover a 503 on the profile read (error, profile: null), a 409 on a profile write (error plus the current list), and a 503 on a human action and on the human read. All three fail without the fix. 224/224 server tests, lint, prettier and the vite build pass locally.

@Mygod

Mygod commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Profile switches can cause v2 deletions to silently do nothing, and v1 authentication failures are misreported as success. Focused form/schema tests and changed-file lint/format checks passed; HTTP-backed tests were blocked by sandbox listener restrictions.

Full review comments:

  • [P2] Retain the original profile for v2 UID mutations — server/src/services/poracle/trackingV2.js:85-87
    If a schedule or bot command switches profiles after the rule list is loaded, these requests omit profile, so PoracleNG scopes them to the newly active profile. Editing an old-profile rule returns 404; deleting it silently succeeds without deleting anything, and the refreshed list hides the still-existing rule. Preserve the loaded rules’ profile context and pass ?profile=N for both edits and deletions.

  • [P2] Reject PoracleJS authentication failure envelopes — server/src/services/poracle/http.js:98-103
    When a PoracleJS secret is incorrect or rotated, its API returns HTTP 200 with status: 'authError' and a reason. This check accepts that response, so createTrackingV1.list() converts the missing category into [] and #runTracking() reports success even for rejected mutations. An in-process reproduction returned {status: 'ok', pokemon: []}. Recognize authentication and IP-rejection envelopes as failures and propagate their reason instead of clearing the list and reporting success.

Fabio1988 and others added 5 commits November 4, 2026 20:50
ReactMap now works with Poracle tracking rules in the PoracleNG v2 shape
end to end: GraphQL rule lists, the webhook store, the editor and the
saved rules all use v2 fields (null means any, named enums, separate
edit/summary booleans). Servers that only speak the legacy v1 API are
served by an adapter that converts rules to and from v1 rows.

- server: poracle/http (problem+json and v1 error messages, generic
  transport errors), trackingV2 (batch POST, PUT keeps uid, bulk
  delete), trackingV1 over v1Adapter
- new incident category (gold stop, kecleon, showcase) on v2
- edit and summary switches shown only when the server supports them
- rule settings ReactMap has no input for are kept on edit
- gym battle_changes exposed, ping and stepped profile schedules kept
- quick gym errors shown instead of a success toast

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nt search label

- `all` is set by the selector tile and is not a rule field; v2 rejects it
- PoracleJS reports some failures as 200 { status: 'error' }; treat them as
  errors again instead of an empty list
- add the missing search_incidents label

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…slation fallbacks

- mutation results update the tracked query's cache, so the active search
  still applies and list rows and tiles index the same rules
- Manage uses the shared result handling
- unknown incident types use the declared incident_unknown label and the
  default template label has no fallback string

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ad errors

- an untouched slider keeps a stored null (e.g. no pvp rank ceiling)
  instead of saving the default it displays
- switching between distance and areas clears the override the new mode
  can't hold (location label needs distance, areas exclude it)
- apply-to-all honours an explicit All Forms
- a failed tracking read returns no list; ALL_PROFILES selects status and
  message, so the client shows the error and keeps the rules it had
- on v1, incident rules use the invasion templates
- the invasion disable flag (and v1 invasion block) also covers incidents

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-all forms

- rule uids, profile numbers and coordinates from the client must be
  numeric before they go into a Poracle URL; unknown categories and
  profile actions are refused instead of being sent as paths
- the apply-to-all preset starts with All Forms off (an absent form no
  longer reads as "all forms")
- the store gets the filtered rows also while a failed read keeps the
  last list, so list and tiles stay on the same rules; the error alert
  shows once per read result
- named labels (team, gender, rsvp) have no fallback string; add the
  missing genderless label

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fabio1988 and others added 3 commits November 4, 2026 20:50
The v1 adapter dropped profile_no, so a pokemon edit was posted without
it. PoracleJS and PoracleNG v1 update pokemon rows by uid and file a
missing profile_no under the active profile, so a profile switch during
the edit moved the rule. The rule now carries the profile_no it was read
with; new rules still get the active profile.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The v2 human and profile calls went through fetchJson, which returns the
failed response instead of throwing, so a 503 on the profile list read as
an empty list. They now use the Poracle transport and come back as
status: 'error' without the list or human, so the client keeps what it
shows and displays the error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- webhookCategories passed the { human } wrapper's blocked_alerts, which
  is always undefined, so blocked alerts never hid a tab; an invasion
  block now hides incidents on v2 too, and a monster block hides pokemon
- apply-to-all on incidents with nothing selected goes to every event
  tile instead of a '0-0' rule without display_type
- the fallback size label treats null bounds as any size
- init settles every request before it returns and tracking is rebuilt
  right after API detection, so a half-failed init can't leave human
  calls on v2 and tracking on v1

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants