fix: clear price to null when a comparison finds no merchant offer
Several memory sticks (e.g. Crucial CT8G48C40U5, Patriot Signature Line,
Kingston FURY Beast) kept displaying their pc-builder.io catalog price
("€35.99", "€39.99"...) even after clicking "Compare prices" and getting
"No live price found, using catalog price" - an unconfirmed number was
being shown as if it were still valid, which is misleading given
pc-builder.io's catalog is known to go stale (see the earlier RTX 5070
€128 vs €66.89 case).
New clearPriceIfNoOffers(): when ScrapedPartsController#compare finds
zero offers across all 7 supported merchants, the part's price/priceSource
are reset to null, regardless of whether the price previously came from
the pc-builder.io catalog or an earlier verified merchant match - per
explicit product decision, an empty comparison always wins over keeping a
possibly-stale number around.
- ScrapedPartsController#compare now calls clearPriceIfNoOffers() in the
empty-offers branch, and returns the resulting price in the JSON
response so the frontend can mirror it exactly
- NewPartSearch.vue: comparePrices() now sets part.price from the
response's `price` field (mirroring exactly what the backend just
persisted) instead of only updating it on a cheaper find; removed the
now-inaccurate "using catalog price €X" message since that price no
longer exists once compared with no result
- 3 new tests for clearPriceIfNoOffers: clears an unverified
pc-builder.io price, also clears a previously-verified merchant price
when no longer confirmed, and is a no-op when there was no price to
begin with
Verified end-to-end against the reported Crucial CT8G48C40U5 example:
price went from 35.99 (pc-builder.io) to null after a real comparison
found zero matching offers on any merchant.
This commit is contained in:
1 parent
fa74791c92
commit
0ce6a7f4a7
4 files changed
+96
-19
No files matched your search
@@ -2,7 +2,11 @@ import type { HttpContext } from '@adonisjs/core/http'
|
|||||||
import vine from '@vinejs/vine'
|
import vine from '@vinejs/vine'
|
||||||
import ScrapedPart from '#models/scraped_part'
|
import ScrapedPart from '#models/scraped_part'
|
||||||
import ComponentType from '#models/component_type'
|
import ComponentType from '#models/component_type'
|
||||||
import { ensureFreshCategory, updatePriceIfCheaper } from '#services/part_scraper_service'
|
import {
|
||||||
|
ensureFreshCategory,
|
||||||
|
updatePriceIfCheaper,
|
||||||
|
clearPriceIfNoOffers,
|
||||||
|
} from '#services/part_scraper_service'
|
||||||
import { findCheapestOffers } from '#services/merchant_price_service'
|
import { findCheapestOffers } from '#services/merchant_price_service'
|
||||||
|
|
||||||
const SEARCH_RESULTS_LIMIT = 150
|
const SEARCH_RESULTS_LIMIT = 150
|
||||||
@@ -76,6 +80,11 @@ export default class ScrapedPartsController {
|
|||||||
* (or no price was known yet), it's persisted as the new default price
|
* (or no price was known yet), it's persisted as the new default price
|
||||||
* (see updatePriceIfCheaper) — so this comparison benefits every future
|
* (see updatePriceIfCheaper) — so this comparison benefits every future
|
||||||
* search/import for this part, not just the current user.
|
* search/import for this part, not just the current user.
|
||||||
|
*
|
||||||
|
* If NO merchant confirms a price at all, the part's price is cleared
|
||||||
|
* to null (see clearPriceIfNoOffers) rather than silently keeping a
|
||||||
|
* pc-builder.io catalog price the user just explicitly asked to verify
|
||||||
|
* and that no longer checks out anywhere.
|
||||||
*/
|
*/
|
||||||
async compare({ request, auth, response }: HttpContext) {
|
async compare({ request, auth, response }: HttpContext) {
|
||||||
await auth.authenticate()
|
await auth.authenticate()
|
||||||
@@ -91,8 +100,10 @@ export default class ScrapedPartsController {
|
|||||||
|
|
||||||
if (offers.length > 0) {
|
if (offers.length > 0) {
|
||||||
await updatePriceIfCheaper(part, offers[0])
|
await updatePriceIfCheaper(part, offers[0])
|
||||||
|
} else {
|
||||||
|
await clearPriceIfNoOffers(part)
|
||||||
}
|
}
|
||||||
|
|
||||||
return response.json({ offers })
|
return response.json({ offers, price: part.price })
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -415,12 +415,34 @@ async function updatePriceIfCheaper(
|
|||||||
await part.save()
|
await part.save()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Clears a part's price when a live comparison found no matching offer on
|
||||||
|
* any supported merchant (see ScrapedPartsController#compare), regardless
|
||||||
|
* of where the price previously came from (pc-builder.io catalog or an
|
||||||
|
* earlier verified merchant match). The pc-builder.io catalog price this
|
||||||
|
* part started with is frequently stale (see shouldKeepExistingPrice's
|
||||||
|
* doc for a confirmed example: pc-builder.io showed €128 for a CPU
|
||||||
|
* actually selling for €66.89) — once a user explicitly asks to verify a
|
||||||
|
* price and no merchant confirms it anymore, keeping the old number
|
||||||
|
* around is misleading, so it's dropped to null instead of silently
|
||||||
|
* falling back to "using catalog price €X".
|
||||||
|
*/
|
||||||
|
async function clearPriceIfNoOffers(part: ScrapedPart): Promise<void> {
|
||||||
|
if (part.price === null) return
|
||||||
|
|
||||||
|
part.price = null
|
||||||
|
part.priceSource = null
|
||||||
|
part.priceCheckedAt = DateTime.now()
|
||||||
|
await part.save()
|
||||||
|
}
|
||||||
|
|
||||||
export {
|
export {
|
||||||
scrapeAllCategories,
|
scrapeAllCategories,
|
||||||
scrapeComponentType,
|
scrapeComponentType,
|
||||||
ensureFreshCategory,
|
ensureFreshCategory,
|
||||||
isCategoryStale,
|
isCategoryStale,
|
||||||
updatePriceIfCheaper,
|
updatePriceIfCheaper,
|
||||||
|
clearPriceIfNoOffers,
|
||||||
shouldKeepExistingPrice,
|
shouldKeepExistingPrice,
|
||||||
CATEGORY_CONFIG,
|
CATEGORY_CONFIG,
|
||||||
CATEGORY_SLUGS,
|
CATEGORY_SLUGS,
|
||||||
|
|||||||
@@ -62,21 +62,20 @@ async function comparePrices(part: ScrapedPart) {
|
|||||||
}
|
}
|
||||||
const data = await response.json()
|
const data = await response.json()
|
||||||
offers[part.id] = data.offers ?? []
|
offers[part.id] = data.offers ?? []
|
||||||
|
|
||||||
|
// The backend already persists this exact value server-side (see
|
||||||
|
// ScrapedPartsController#compare): the cheapest offer's price when
|
||||||
|
// one was found (updatePriceIfCheaper), or null when no merchant
|
||||||
|
// confirmed a price at all (clearPriceIfNoOffers) — an unverified
|
||||||
|
// pc-builder.io catalog price is not kept around just because the
|
||||||
|
// comparison came back empty. Mirror that here on the locally held
|
||||||
|
// result so the display (e.g. "€979.95" or "No price yet") reflects
|
||||||
|
// it immediately, instead of the initial search-result price sticking
|
||||||
|
// around until the user re-runs the search from scratch.
|
||||||
|
part.price = data.price ?? null
|
||||||
|
|
||||||
if (offers[part.id].length === 0) {
|
if (offers[part.id].length === 0) {
|
||||||
compareErrors[part.id] = true
|
compareErrors[part.id] = true
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
// The backend already persists the cheapest offer found as this
|
|
||||||
// part's new best-known price when it beats the catalog price (see
|
|
||||||
// ScrapedPartsController#compare / updatePriceIfCheaper). Mirror that
|
|
||||||
// here on the locally held result so the displayed catalog price
|
|
||||||
// (e.g. "€979.95") reflects it immediately, instead of staying stuck
|
|
||||||
// at whatever price the initial search returned until the user
|
|
||||||
// re-runs the search from scratch.
|
|
||||||
const cheapest = offers[part.id][0]
|
|
||||||
if (part.price === null || cheapest.price < part.price) {
|
|
||||||
part.price = cheapest.price
|
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
compareErrors[part.id] = true
|
compareErrors[part.id] = true
|
||||||
@@ -185,10 +184,7 @@ function importPart(part: ScrapedPart) {
|
|||||||
</a>
|
</a>
|
||||||
</template>
|
</template>
|
||||||
|
|
||||||
<span v-if="compareErrors[part.id] && part.price !== null" class="text-xs text-gray-400">
|
<span v-if="compareErrors[part.id]" class="text-xs text-gray-400">
|
||||||
No live price found, using catalog price €{{ part.price.toFixed(2) }}
|
|
||||||
</span>
|
|
||||||
<span v-else-if="compareErrors[part.id]" class="text-xs text-gray-400">
|
|
||||||
No price found on any merchant for this part.
|
No price found on any merchant for this part.
|
||||||
</span>
|
</span>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import {
|
|||||||
isCategoryStale,
|
isCategoryStale,
|
||||||
ensureFreshCategory,
|
ensureFreshCategory,
|
||||||
updatePriceIfCheaper,
|
updatePriceIfCheaper,
|
||||||
|
clearPriceIfNoOffers,
|
||||||
shouldKeepExistingPrice,
|
shouldKeepExistingPrice,
|
||||||
CACHE_TTL_HOURS,
|
CACHE_TTL_HOURS,
|
||||||
} from '#services/part_scraper_service'
|
} from '#services/part_scraper_service'
|
||||||
@@ -217,6 +218,53 @@ test.group('Best-known-price persistence', () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test.group('clearPriceIfNoOffers', () => {
|
||||||
|
test('clears an unverified pc-builder.io catalog price when no merchant confirms it', async ({
|
||||||
|
assert,
|
||||||
|
}) => {
|
||||||
|
// Regression: memory sticks kept showing pc-builder.io catalog prices
|
||||||
|
// (e.g. €35.99) even though comparing prices found no matching offer
|
||||||
|
// on any merchant ("No live price found, using catalog price €35.99")
|
||||||
|
// — an unconfirmed number should not be displayed as if it were still
|
||||||
|
// valid.
|
||||||
|
const type = await createType('Memory-clear-1')
|
||||||
|
const part = await createScrapedPart(type.id, 'ZZZ-Test-Clear-A', 'clear-a-p1', 35.99)
|
||||||
|
|
||||||
|
await clearPriceIfNoOffers(part)
|
||||||
|
await part.refresh()
|
||||||
|
|
||||||
|
assert.isNull(part.price)
|
||||||
|
assert.isNull(part.priceSource)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('also clears a price previously verified by a merchant if no longer confirmed', async ({
|
||||||
|
assert,
|
||||||
|
}) => {
|
||||||
|
const type = await createType('Memory-clear-2')
|
||||||
|
const part = await createScrapedPart(type.id, 'ZZZ-Test-Clear-B', 'clear-b-p1', 50)
|
||||||
|
part.priceSource = 'LDLC'
|
||||||
|
await part.save()
|
||||||
|
|
||||||
|
await clearPriceIfNoOffers(part)
|
||||||
|
await part.refresh()
|
||||||
|
|
||||||
|
assert.isNull(part.price)
|
||||||
|
assert.isNull(part.priceSource)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('is a no-op when the part already has no price', async ({ assert }) => {
|
||||||
|
const type = await createType('Memory-clear-3')
|
||||||
|
const part = await createScrapedPart(type.id, 'ZZZ-Test-Clear-C', 'clear-c-p1', null)
|
||||||
|
const originalCheckedAt = part.priceCheckedAt
|
||||||
|
|
||||||
|
await clearPriceIfNoOffers(part)
|
||||||
|
await part.refresh()
|
||||||
|
|
||||||
|
assert.isNull(part.price)
|
||||||
|
assert.equal(part.priceCheckedAt, originalCheckedAt)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
test.group('Catalog refresh price protection (shouldKeepExistingPrice)', () => {
|
test.group('Catalog refresh price protection (shouldKeepExistingPrice)', () => {
|
||||||
test('an unverified pc-builder.io price is NOT protected and gets overwritten', ({ assert }) => {
|
test('an unverified pc-builder.io price is NOT protected and gets overwritten', ({ assert }) => {
|
||||||
// Regression test: a GPU had price=431.17 from a previous pc-builder.io
|
// Regression test: a GPU had price=431.17 from a previous pc-builder.io
|
||||||
|
|||||||
Reference in new issue
Block a user