Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/componen
import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component';
import { WhereChipComponent } from '../../shared/components/where-chip/where-chip.component';
import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component';
import { orderAlarms } from '../../shared/utils/alarm-order';
import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope';

@Component({
Expand Down Expand Up @@ -249,7 +250,7 @@ export class FortChangeListComponent implements OnInit {
.subscribe({
error: () => this.loading.set(false),
next: items => {
this.fortChanges.set(items);
this.fortChanges.set(orderAlarms(items, f => [f.fortType, f.changeTypes.join(',')]));
this.loading.set(false);
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ import { ScannerService } from '../../core/services/scanner.service';
import { TestAlertService } from '../../core/services/test-alert.service';
import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/components/confirm-dialog/confirm-dialog.component';
import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component';
import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component';
import { QuietChipComponent } from '../../shared/components/quiet-chip/quiet-chip.component';
import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component';
import { WhereChipComponent } from '../../shared/components/where-chip/where-chip.component';
import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component';
import { orderAlarms } from '../../shared/utils/alarm-order';
import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope';

@Component({
Expand Down Expand Up @@ -264,7 +265,7 @@ export class GymListComponent implements OnInit {
.subscribe({
error: () => this.loading.set(false),
next: g => {
this.gyms.set(g);
this.gyms.set(orderAlarms(g, x => [x.team, x.gymId]));
this.loading.set(false);
this.resolveGymNames(g);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { DistanceDialogComponent } from '../../shared/components/distance-dialog
import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component';
import { WhereChipComponent } from '../../shared/components/where-chip/where-chip.component';
import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component';
import { orderAlarms } from '../../shared/utils/alarm-order';
import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope';

@Component({
Expand Down Expand Up @@ -272,7 +273,7 @@ export class LureListComponent implements OnInit {
.subscribe({
error: () => this.loading.set(false),
next: l => {
this.lures.set(l);
this.lures.set(orderAlarms(l, x => [x.lureId]));
this.loading.set(false);
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ import { MaxBattleService } from '../../core/services/max-battle.service';
import { AlarmInfoComponent } from '../../shared/components/alarm-info/alarm-info.component';
import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/components/confirm-dialog/confirm-dialog.component';
import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component';
import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component';
import { QuietChipComponent } from '../../shared/components/quiet-chip/quiet-chip.component';
import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component';
import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component';
import { orderAlarms } from '../../shared/utils/alarm-order';
import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope';

@Component({
Expand Down Expand Up @@ -297,7 +298,7 @@ export class MaxBattleListComponent implements OnInit {
this.loading.set(false);
},
next: maxBattles => {
this.maxBattles.set(maxBattles);
this.maxBattles.set(orderAlarms(maxBattles, m => [m.pokemonId, m.level, m.form, m.stationId]));
this.loading.set(false);
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ import { NestService } from '../../core/services/nest.service';
import { TestAlertService } from '../../core/services/test-alert.service';
import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/components/confirm-dialog/confirm-dialog.component';
import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component';
import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component';
import { QuietChipComponent } from '../../shared/components/quiet-chip/quiet-chip.component';
import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component';
import { WhereChipComponent } from '../../shared/components/where-chip/where-chip.component';
import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component';
import { orderAlarms } from '../../shared/utils/alarm-order';
import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope';
import { isAutoDelete as cleanIsAutoDelete } from '../../shared/utils/clean-flags';

Expand Down Expand Up @@ -240,7 +241,7 @@ export class NestListComponent implements OnInit {
.subscribe({
error: () => this.loading.set(false),
next: n => {
this.nests.set(n);
this.nests.set(orderAlarms(n, x => [x.pokemonId, x.minSpawnAvg]));
this.loading.set(false);
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,22 @@ describe('PokestopEventListComponent', () => {
{ ...base, uid: 3, displayType: GOLD_STOP, eventName: 'gold-stop' },
]);

expect(cardTitles()).toEqual(['INVASIONS.EVENT_TYPES.SHOWCASE', 'INVASIONS.EVENT_TYPES.KECLEON', 'INVASIONS.EVENT_TYPES.GOLD_STOP']);
// Ordered by display type rather than by the order PoracleNG returned them in.
expect(cardTitles()).toEqual(['INVASIONS.EVENT_TYPES.GOLD_STOP', 'INVASIONS.EVENT_TYPES.KECLEON', 'INVASIONS.EVENT_TYPES.SHOWCASE']);
});

it('keeps a rule where it was after an edit rotated its uid', () => {
// This type has only ever had a v2 write surface, and a v2 replace is delete-then-insert, so the
// edited rule comes back under the highest uid in the list. Rendering PoracleNG's own order threw
// the card the user had just saved to the end of the grid.
setup([
// PoracleNG's own order: by uid, with the just-edited Kecleon rule re-keyed to the highest.
{ ...base, uid: 1, displayType: GOLD_STOP, eventName: 'gold-stop' },
{ ...base, uid: 3, displayType: SHOWCASE },
{ ...base, uid: 99, displayType: KECLEON, eventName: 'kecleon' },
]);

expect(cardTitles()).toEqual(['INVASIONS.EVENT_TYPES.GOLD_STOP', 'INVASIONS.EVENT_TYPES.KECLEON', 'INVASIONS.EVENT_TYPES.SHOWCASE']);
});

it('shows the empty state, and no cards, when the profile tracks nothing', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/componen
import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component';
import { WhereChipComponent } from '../../shared/components/where-chip/where-chip.component';
import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component';
import { orderAlarms } from '../../shared/utils/alarm-order';
import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope';
import { isAutoDelete } from '../../shared/utils/clean-flags';
import { pokestopEventInfo } from '../../shared/utils/pokestop-events';
Expand Down Expand Up @@ -241,7 +242,7 @@ export class PokestopEventListComponent implements OnInit {
.subscribe({
error: () => this.loading.set(false),
next: items => {
this.events.set(items);
this.events.set(orderAlarms(items, e => [e.displayType]));
this.loading.set(false);
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { ConfirmDialogComponent, ConfirmDialogData } from '../../shared/componen
import { DistanceDialogComponent } from '../../shared/components/distance-dialog/distance-dialog.component';
import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component';
import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component';
import { orderAlarms } from '../../shared/utils/alarm-order';
import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope';

@Component({
Expand Down Expand Up @@ -316,7 +317,7 @@ export class QuestListComponent implements OnInit {
this.loading.set(false);
},
next: quests => {
this.quests.set(quests);
this.quests.set(orderAlarms(quests, q => [q.rewardType, q.reward, q.amount]));
this.loading.set(false);
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { RsvpPillComponent } from '../../shared/components/rsvp-pill/rsvp-pill.c
import { RuleSummaryComponent } from '../../shared/components/rule-summary/rule-summary.component';
import { WhereSheetComponent, WhereSheetData } from '../../shared/components/where-sheet/where-sheet.component';
import { LevelLabelPipe } from '../../shared/pipes/level-label.pipe';
import { orderAlarms } from '../../shared/utils/alarm-order';
import { AlarmScope, scopeOf, scopeToFields } from '../../shared/utils/alarm-scope';
import { NO_COSTUME } from '../../shared/utils/costumes';

Expand Down Expand Up @@ -406,8 +407,8 @@ export class RaidListComponent implements OnInit {
this.loading.set(false);
},
next: ([raids, eggs]) => {
this.raids.set(raids);
this.eggs.set(eggs);
this.raids.set(orderAlarms(raids, r => [r.pokemonId, r.level, r.form, r.gymId]));
this.eggs.set(orderAlarms(eggs, e => [e.level, e.team, e.gymId]));
this.loading.set(false);
this.resolveGymNames([...raids, ...eggs]);
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { orderAlarms } from './alarm-order';

describe('orderAlarms', () => {
it('keeps a rule where it was after an edit gave it a new id', () => {
// The regression this exists for. PoracleNG 5.2.0 replaces a rule rather than updating it, so the
// edited row comes back with the highest id in the list and the card jumped to the end of the grid.
const before = [
{ level: 5, pokemonId: 25, uid: 10 },

Check warning on line 8 in Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.spec.ts

View workflow job for this annotation

GitHub Actions / Frontend (Angular)

Expected "uid" (top) to come before "pokemonId" (unknown)
{ level: 5, pokemonId: 150, uid: 11 },

Check warning on line 9 in Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.spec.ts

View workflow job for this annotation

GitHub Actions / Frontend (Angular)

Expected "uid" (top) to come before "pokemonId" (unknown)
{ level: 5, pokemonId: 380, uid: 12 },

Check warning on line 10 in Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.spec.ts

View workflow job for this annotation

GitHub Actions / Frontend (Angular)

Expected "uid" (top) to come before "pokemonId" (unknown)
];
const afterEditingMewtwo = [
{ level: 5, pokemonId: 25, uid: 10 },

Check warning on line 13 in Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.spec.ts

View workflow job for this annotation

GitHub Actions / Frontend (Angular)

Expected "uid" (top) to come before "pokemonId" (unknown)
{ level: 5, pokemonId: 380, uid: 12 },

Check warning on line 14 in Applications/Pgan.PoracleWebNet.App/ClientApp/src/app/shared/utils/alarm-order.spec.ts

View workflow job for this annotation

GitHub Actions / Frontend (Angular)

Expected "uid" (top) to come before "pokemonId" (unknown)
{ level: 5, pokemonId: 150, uid: 99 },
];

const key = (r: { level: number; pokemonId: number }) => [r.pokemonId, r.level];

expect(orderAlarms(afterEditingMewtwo, key).map(r => r.pokemonId)).toEqual(orderAlarms(before, key).map(r => r.pokemonId));
expect(orderAlarms(afterEditingMewtwo, key)[1].uid).toBe(99);
});

it('falls back to the id so the order is total', () => {
const items = [{ uid: 3 }, { uid: 1 }, { uid: 2 }];

expect(orderAlarms(items, () => []).map(i => i.uid)).toEqual([1, 2, 3]);
});

it('compares numbers as numbers and strings as strings', () => {
const items = [
{ uid: 1, value: 10 },
{ uid: 2, value: 9 },
];

expect(orderAlarms(items, i => [i.value]).map(i => i.uid)).toEqual([2, 1]);
expect(
orderAlarms(
[
{ name: 'b', uid: 1 },
{ name: 'a', uid: 2 },
],
i => [i.name],
).map(i => i.uid),
).toEqual([2, 1]);
});

it('treats a null or absent key as the empty string rather than throwing', () => {
// gymId, stationId and fortType are all nullable, and "any gym" is the common case.
const items = [
{ gymId: 'abc', uid: 1 },
{ gymId: null, uid: 2 },
];

expect(orderAlarms(items, i => [i.gymId]).map(i => i.uid)).toEqual([2, 1]);
});

it('does not mutate the list it was given', () => {
const items = [{ uid: 3 }, { uid: 1 }];

orderAlarms(items, () => []);

expect(items.map(i => i.uid)).toEqual([3, 1]);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/**
* A stable display order for an alarm list, independent of the rule id.
*
* PoracleNG returns tracking rows in id order, and nine of the eleven lists rendered that order straight
* through. On PoracleNG 5.2.0 and newer an edit is a replace: the rule comes back under a new, higher id,
* so the card the user just saved jumped to the end of the grid and, on a long list, off the screen —
* with nothing to say it had moved.
*
* Ordering on the rule's own content instead keeps a card where it was, and is a better order than
* insertion sequence regardless. The keys are the fields the card is titled by, in their raw form: dex
* number rather than species name, reward type rather than reward name. Sorting on the resolved name
* would mean re-ordering the grid once the master data and the gym names arrive, which is a worse flicker
* than the problem being fixed — and the resolved name changes with the display language, so the order
* would too.
*
* Invasion is deliberately absent: it stays on PoracleNG's v1 write surface, so its ids do not rotate. The
* pokemon list already sorts on its own controls. Pokestop events are here too — that type has only ever
* had a v2 surface, so its ids have rotated on every edit since it shipped.
*/
export function orderAlarms<T extends { uid: number }>(
items: readonly T[],
key: (item: T) => readonly (number | string | null | undefined)[],
): T[] {
return [...items].sort((a, b) => {
const left = key(a);
const right = key(b);

for (let i = 0; i < left.length; i++) {
const difference = compare(left[i], right[i]);
if (difference !== 0) {
return difference;
}
}

// Two rules that are alike in everything the card shows. The id is the last tiebreak, so the order is
// total and a re-render cannot shuffle them against each other.
return a.uid - b.uid;
});
}

function compare(a: number | string | null | undefined, b: number | string | null | undefined): number {
if (typeof a === 'number' && typeof b === 'number') {
return a - b;
}

return String(a ?? '').localeCompare(String(b ?? ''));
}
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- **Nothing on screen: the last code that could write an alarm straight to Poracle's database has been taken out.** Every alarm write has gone through PoracleNG's API since 2.0, so its deduplication, its field defaults and its immediate state reload all run -- but the database tables were still mapped in code beside it, one line away from being used again. That mapping is gone, along with a set of profile methods nothing had called since the same migration. The two places that still reach the alarm tables directly are unchanged and are there for reasons written down beside them.

- **A refused alarm explains itself the same way whichever Poracle surface answered.** The v2 write path already read PoracleNG's newer RFC 9457 error bodies and named the individual field it refused; the older v1 path, still the one most installs use, was reading only the older shape and answering a validation refusal as though the server had broken. Both paths now share one reader, and where many fields are refused at once the message names the first few and counts the rest rather than rendering a dozen clauses into a snackbar ([#803](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/803)).
- **Alarm lists keep a card where it was after you edit it.** Raids, eggs, quests, lures, nests, gyms, max battles and fort changes were rendered in the order Poracle returned them, which is the order the rules were created. On PoracleNG 5.2.0 and newer an edit replaces the rule rather than updating it, so the card you had just saved would have jumped to the end of the grid — off the screen entirely on a long list, with nothing to say it had moved. Each of those lists now has its own order, on what the card is titled by: Pokémon and level for raids and max battles, level for eggs, reward for quests, lure type, species for nests, team for gyms, change type for fort changes, event type for Pokéstop events. Pokéstop events are in that list for a different reason: that type has only ever had the newer write surface, so its cards have been jumping since it shipped. The Pokemon list already sorted itself and is unchanged.
- **Eight more alarm types have their edits written through PoracleNG 5.2.1's strict `/api/v2` surface, and lure edits stop being risky.** Raid, egg, quest, nest, gym, max battle, fort change and lure join Pokemon on the newer write path, which addresses a rule by its id and replaces it in place. For most of them nothing changes on screen. Lure is the exception worth naming: PoracleNG's older surface has no way to update a lure alarm at all, so editing one had to delete the rule, re-create it, and put the original back if that failed — a sequence with a window in which the alarm did not exist. Max battle edits carried the same window. Neither does now. Invasion alarms stay on the older surface deliberately: the new one cannot report which grunt a rule targets, so an edit could not be written back faithfully. Anything older than PoracleNG 5.2.0 keeps the path it has always used, unchanged, and an edit carrying something the new surface cannot express — a role mention, an egg with no level, a fort rule missing its empty-changes setting — takes the old path rather than failing. Set `PORACLE_TRACKING_API_VERSION` to `v1` or `v2` to pin it.
- **Pokemon alarm edits are written through PoracleNG 5.2.1’s strict `/api/v2` surface, where the server can tell an edit apart from a takeover.** Nothing changes on screen. What changes is underneath: an edit now addresses the rule by its id, so Poracle refuses outright if the uid is not yours or if the result would duplicate an alarm you already have, instead of PoracleWeb.NET having to work that out from a success response and undo it afterwards. Poracle also explains a rejected filter field by name now, so the message on the dialog says which one. Anything older than 5.2.0 keeps the surface it has always used, unchanged, and so do the other nine alarm types; an edit carrying anything the new surface cannot express takes the old path rather than failing. Set `PORACLE_TRACKING_API_VERSION` to `v1` or `v2` to pin it ([#805](https://github.com/PGAN-Dev/PoracleWeb.NET/issues/805)).

### Fixed
Expand Down
Loading
Loading