Reviews: a store read that failed everywhere is an error, not an empty fresh list; Reviews tab says Apple's API can lag and Play returns 7 days; pitfall 106
This commit is contained in:
parent
0e88e4293a
commit
75bf62e125
7 changed files with 50 additions and 8 deletions
|
|
@ -366,6 +366,7 @@ export const en = {
|
|||
'reviews.shown': '{count} shown',
|
||||
'reviews.noMatch': 'No review matches the filter',
|
||||
'reviews.pick': 'Pick a review',
|
||||
'reviews.lagNote': "Read from the stores' APIs. Apple's API can trail the App Store Connect page; Google Play returns only the last 7 days.",
|
||||
'reviews.answered': 'Answered',
|
||||
'reviews.drafted': 'Draft',
|
||||
'reviews.approved': 'Approved',
|
||||
|
|
|
|||
|
|
@ -368,6 +368,7 @@ export const tr: Record<keyof typeof en, string> = {
|
|||
'reviews.shown': '{count} gösteriliyor',
|
||||
'reviews.noMatch': 'Süzgece uyan yorum yok',
|
||||
'reviews.pick': 'Bir yorum seçin',
|
||||
'reviews.lagNote': "Mağaza API'lerinden okunur. Apple'ın API'si App Store Connect sayfasından geride kalabilir; Google Play yalnızca son 7 günü verir.",
|
||||
'reviews.answered': 'Yanıtlandı',
|
||||
'reviews.drafted': 'Taslak',
|
||||
'reviews.approved': 'Onaylı',
|
||||
|
|
|
|||
|
|
@ -135,6 +135,7 @@ export function ReviewsTab({ app }: { app: UnifiedApp }) {
|
|||
<div className="rv2-side">
|
||||
<div className="rv2-unans"><b className="num">{num(stats.unanswered)}</b><span>{t('reviews.unansweredN', { count: stats.unanswered })}</span></div>
|
||||
<ReadControl r={list} />
|
||||
<p className="small muted">{t("reviews.lagNote")}</p>
|
||||
{project && <WriteButton run={w} icon="text" hint={t('reviews.makeDraftsHint')} w={{ method: 'reviews.draft', args: { days: 30 }, title: t('reviews.makeDrafts'), done: t('reviews.draftsMade'), after: changed }}>{t('reviews.makeDrafts')}</WriteButton>}
|
||||
</div>
|
||||
</section>
|
||||
|
|
|
|||
|
|
@ -63,6 +63,19 @@ export function parsePlayReviews(resp: PlayReviewsResp): Review[] {
|
|||
});
|
||||
}
|
||||
|
||||
export interface StoreRead { store: 'apple' | 'play'; reviews?: Review[]; error?: string }
|
||||
|
||||
/**
|
||||
* Combines the per-store reads of `reviews list`. A store that failed is reported (warning text per store) but does not
|
||||
* hide the other one; when EVERY store that was asked failed it throws, so a broken key never reads as "0 reviews, all
|
||||
* fine" (the desktop kept such an empty result as a fresh read and showed no new reviews).
|
||||
*/
|
||||
export function settleStoreReads(reads: StoreRead[]): { reviews: Review[]; errors: string[] } {
|
||||
const errors = reads.filter(r => r.error !== undefined).map(r => `${r.store}: ${r.error}`);
|
||||
if (reads.length && errors.length === reads.length) throw new Error(errors.join('; '));
|
||||
return { reviews: reads.flatMap(r => r.reviews ?? []), errors };
|
||||
}
|
||||
|
||||
/** Reviews newer than `sinceIso`, optional filters, newest first. */
|
||||
export function filterReviews(rs: Review[], o: { since?: string; unanswered?: boolean; ratings?: number[] } = {}): Review[] {
|
||||
return rs.filter(r => (!o.since || r.date >= o.since) && (!o.unanswered || !r.reply) && (!o.ratings?.length || o.ratings.includes(r.rating)))
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@ import { confirm } from './lib/ctx';
|
|||
import { done, log, printJson, warn } from './lib/log';
|
||||
import {
|
||||
checkReply, type Draft, draftMarkdown, draftName, filterReviews, markSent, parseAppleReviews, parseDraft, parsePlayReviews, replyRequest,
|
||||
type Review, sendPlan,
|
||||
type Review, sendPlan, settleStoreReads, type StoreRead,
|
||||
} from './lib/reviews';
|
||||
import { textTable } from './lib/table';
|
||||
import { readReplyDrafts, setApproved, setReplyText } from './lib/project-files';
|
||||
|
|
@ -63,13 +63,13 @@ async function collect(ctx: Ctx, o: { since: string; unanswered: boolean; rating
|
|||
const { config } = ctx.loaded;
|
||||
const store = ctx.args.get('store');
|
||||
if (store && store !== 'apple' && store !== 'play') throw new Error('--store apple | play');
|
||||
const all: Review[] = [];
|
||||
if ((!store || store === 'apple') && config.apple) {
|
||||
try { all.push(...await appleReviews(ctx, o.since)); } catch (e) { if (store) throw e; warn(TAG, `apple: ${(e as Error).message.slice(0, 200)}`); }
|
||||
}
|
||||
if ((!store || store === 'play') && config.play) {
|
||||
try { all.push(...await playReviews(ctx)); } catch (e) { if (store) throw e; warn(TAG, `play: ${(e as Error).message.slice(0, 200)}`); }
|
||||
const reads: StoreRead[] = [];
|
||||
for (const [name, on, read] of [['apple', !!config.apple, () => appleReviews(ctx, o.since)], ['play', !!config.play, () => playReviews(ctx)]] as const) {
|
||||
if (store && store !== name || !on) continue;
|
||||
try { reads.push({ store: name, reviews: await read() }); } catch (e) { if (store) throw e; reads.push({ store: name, error: (e as Error).message.slice(0, 200) }); }
|
||||
}
|
||||
const { reviews: all, errors } = settleStoreReads(reads);
|
||||
for (const e of errors) warn(TAG, e);
|
||||
return filterReviews(all, o);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1272,3 +1272,14 @@ that fails without the fix (see CONTRIBUTING.md).
|
|||
- **Cause:** a JSON config (store-only apps get one generated) was loaded with a bare dynamic import, which Node refuses.
|
||||
- **Do:** update storebound.
|
||||
- **Guard:** `importConfigModule` in `cli/lib/rt.ts` reads JSON configs as data (`tests/config-json-node.test.ts`).
|
||||
|
||||
## 106. Reviews "not up to date": a failed store read looked like "0 new reviews"
|
||||
- **Symptom:** the desktop Reviews tab (and `storebound reviews list`) showed no new reviews although the stores had some.
|
||||
- **Cause:** (1) `reviews list` swallowed a store's error (expired key, wrong app) as a warning and exited 0 with an empty
|
||||
list, which the desktop kept as a fresh read. (2) Google Play's reviews API only returns reviews created or edited in
|
||||
the last 7 days, so older Play reviews vanish from every refresh. (3) The App Store Connect API lists written reviews
|
||||
only (not star-only ratings) and can trail the ASC web page. Checked against Apple's public customer-reviews feed:
|
||||
the API matched it, so the API itself was not stale there.
|
||||
- **Do:** read the "read ... ago" stamp and refresh; for Play keep your own history; compare with the console if in doubt.
|
||||
- **Guard:** `settleStoreReads` (`cli/lib/reviews.ts`) throws when every asked store failed (`tests/reviews.test.ts`);
|
||||
the Reviews tab states the Play 7-day window and that Apple's API can lag.
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import { describe, expect, test } from 'bun:test';
|
|||
import { parseFrontMatter, stringifyFrontMatter } from '../cli/lib/frontmatter';
|
||||
import {
|
||||
checkReply, draftMarkdown, draftName, filterReviews, markSent, parseAppleReviews, parseDraft, parsePlayReviews, REPLY_LIMITS, REPLY_PLACEHOLDER,
|
||||
replyRequest, sendPlan,
|
||||
replyRequest, sendPlan, settleStoreReads,
|
||||
} from '../cli/lib/reviews';
|
||||
import { FORBIDDEN_ARGS, TOOLS, toArgv } from '../cli/mcp-tools';
|
||||
|
||||
|
|
@ -104,3 +104,18 @@ describe('reviews', () => {
|
|||
expect(TOOLS.some(x => x.command.includes('send'))).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('settleStoreReads', () => {
|
||||
const r = { store: 'apple' as const, id: '1', rating: 5, body: 'x', date: '2026-10-01T00:00:00.000Z' };
|
||||
test('one failed store does not hide the other, and is reported', () => {
|
||||
const o = settleStoreReads([{ store: 'apple', reviews: [r] }, { store: 'play', error: 'denied' }]);
|
||||
expect(o.reviews).toEqual([r]);
|
||||
expect(o.errors).toEqual(['play: denied']);
|
||||
});
|
||||
test('every asked store failed: throws instead of an empty "all fine" list', () => {
|
||||
expect(() => settleStoreReads([{ store: 'apple', error: 'expired key' }])).toThrow('apple: expired key');
|
||||
});
|
||||
test('no store asked: empty, no error', () => {
|
||||
expect(settleStoreReads([])).toEqual({ reviews: [], errors: [] });
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue