Development#1216
Conversation
Bumps [io.freefair.lombok](https://github.com/freefair/gradle-plugins) from 9.2.0 to 9.4.0. - [Release notes](https://github.com/freefair/gradle-plugins/releases) - [Commits](freefair/gradle-plugins@9.2.0...9.4.0) --- updated-dependencies: - dependency-name: io.freefair.lombok dependency-version: 9.4.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [org.jsoup:jsoup](https://github.com/jhy/jsoup) from 1.22.1 to 1.22.2. - [Release notes](https://github.com/jhy/jsoup/releases) - [Changelog](https://github.com/jhy/jsoup/blob/master/CHANGES.md) - [Commits](jhy/jsoup@jsoup-1.22.1...jsoup-1.22.2) --- updated-dependencies: - dependency-name: org.jsoup:jsoup dependency-version: 1.22.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [io.freefair.lombok](https://github.com/freefair/gradle-plugins) from 9.2.0 to 9.4.0. - [Release notes](https://github.com/freefair/gradle-plugins/releases) - [Commits](freefair/gradle-plugins@9.2.0...9.4.0) --- updated-dependencies: - dependency-name: io.freefair.lombok dependency-version: 9.4.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [com.deque.html.axe-core:playwright](https://github.com/dequelabs/axe-core-maven-html) from 4.11.1 to 4.11.2. - [Release notes](https://github.com/dequelabs/axe-core-maven-html/releases) - [Changelog](https://github.com/dequelabs/axe-core-maven-html/blob/v4.11.2/CHANGELOG.md) - [Commits](dequelabs/axe-core-maven-html@v4.11.1...v4.11.2) --- updated-dependencies: - dependency-name: com.deque.html.axe-core:playwright dependency-version: 4.11.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [io.freefair.lombok](https://github.com/freefair/gradle-plugins) from 9.2.0 to 9.4.0. - [Release notes](https://github.com/freefair/gradle-plugins/releases) - [Commits](freefair/gradle-plugins@9.2.0...9.4.0) --- updated-dependencies: - dependency-name: io.freefair.lombok dependency-version: 9.4.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
…onsense is organized
…radle/software/core/oqm-core-api/development/io.freefair.lombok-9.4.0 Bump io.freefair.lombok from 9.2.0 to 9.4.0 in /software/core/oqm-core-api
…oragotchi/development/io.freefair.lombok-9.4.0
…radle/software/plugins/storagotchi/development/io.freefair.lombok-9.4.0 Bump io.freefair.lombok from 9.2.0 to 9.4.0 in /software/plugins/storagotchi
…ore-base-station/development/com.deque.html.axe-core-playwright-4.11.2
…radle/software/core/oqm-core-base-station/development/com.deque.html.axe-core-playwright-4.11.2 Bump com.deque.html.axe-core:playwright from 4.11.1 to 4.11.2 in /software/core/oqm-core-base-station
Bumps [io.freefair.lombok](https://github.com/freefair/gradle-plugins) from 9.2.0 to 9.4.0. - [Release notes](https://github.com/freefair/gradle-plugins/releases) - [Commits](freefair/gradle-plugins@9.2.0...9.4.0) --- updated-dependencies: - dependency-name: io.freefair.lombok dependency-version: 9.4.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAhoy, me hearty! This be a substantial overhaul of the external item search plugin's architectural framework—consolidating fragmented lookup endpoints into a unified REST search contract, reorganizing provider implementations under a proper abstract service hierarchy, rewiring the base station's UI clients and REST passthrough interfaces, updating dev service stubs and configuration mappings, and hoisting dependency versions across the fleet. 'Tis a thorough reordering of the rigging, it is. Changes
Sequence DiagramsequenceDiagram
participant UI as UI (Browser/Template)
participant REST as ItemLookupRestInterface
participant Client as ExternalItemSearchClient
participant Service as ExtItemLookupService
participant Provider as ItemSearchService<br/>(e.g., BarcodeLookup)
participant API as External Provider<br/>API
UI->>REST: POST /api/v1/search<br/>(lookupMethod, sources, q)
REST->>Client: search(ItemLookupRequest)
Client->>Service: search(ExtItemSearch)
Service->>Service: getSearchServices()
Service->>Provider: search(method, sources, term)
Provider->>API: REST call<br/>(barcode/query/etc.)
API-->>Provider: JSON response
Provider->>Provider: jsonToResults()<br/>(transform & map)
Provider-->>Service: Multi<LookupResult>
Service->>Service: filter(NO_RESULTS?)<br/>keepNotFound flag
Service-->>Client: Multi<LookupResult>
Client-->>REST: Multi<LookupResult>
REST-->>UI: Stream of<br/>LookupResult (SUCCESS/<br/>ERROR/NO_RESULTS)
UI->>UI: Render table &<br/>detail rows
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
…ternal-item-search/development/io.freefair.lombok-9.4.0
…radle/software/plugins/external-item-search/development/io.freefair.lombok-9.4.0 Bump io.freefair.lombok from 9.2.0 to 9.4.0 in /software/plugins/external-item-search
…ternal-item-search/development/org.jsoup-jsoup-1.22.2
…onfig setting file precedence when reading in
…xt-item-search---rework-interface Plugin - Ext Item Search - interface rework
Code Coverage
|
Code Coverage
|
Code Coverage
|
Code Coverage
|
1 similar comment
Code Coverage
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
software/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/Identifiers.js (1)
58-82:⚠️ Potential issue | 🟠 MajorIdentifier value needs trimming and URL-safe encoding, aye.
Line 63 appends raw user/provider input into a path segment; malformed characters can misroute the request. Line 76 also misses whitespace-only guard.
⚓ Proposed fix
static addIdentifierFromValue(identifierInputContainerJq, newIdentifier){ console.log("Adding a new identifier: ", newIdentifier); + const normalizedIdentifier = (newIdentifier ?? "").trim(); return Rest.call({ failMessagesDiv: identifierInputContainerJq, - url: Rest.passRoot + "/identifier/getIdObject/" + newIdentifier, + url: Rest.passRoot + "/identifier/getIdObject/" + encodeURIComponent(normalizedIdentifier), returnType: "html", extraHeaders: { "accept": "text/html", @@ static addIdentifier(identifierInputContainerJq) { - let newIdentifier = Identifiers.getNewIdentifierValue(identifierInputContainerJq); - if(newIdentifier === "") { + let newIdentifier = (Identifiers.getNewIdentifierValue(identifierInputContainerJq) ?? "").trim(); + if(newIdentifier === "") { console.log("Not adding empty identifier."); return; }software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/InventoryItemUi.java (1)
63-79:⚠️ Potential issue | 🟠 MajorDon’t let the optional ext-search probe sink
/items.A bad
extSearchUrlor a failedallMethodInfo()call will bubble out of page rendering and turn the whole items page into a 500. Since this integration is optional, log the failure and fall back tonull/empty method data instead of making the page depend on that remote service.⚓ Proposed hardening
- public Uni<Response> itemsPage(`@BeanParam` InventoryItemSearch search) throws MalformedURLException { + public Uni<Response> itemsPage(`@BeanParam` InventoryItemSearch search) { boolean extSearchEnabled = !this.extSearchUrl.isBlank(); + Uni<?> extSearchMethods = Uni.createFrom().nullItem(); + if (extSearchEnabled) { + try { + extSearchMethods = QuarkusRestClientBuilder.newBuilder() + .baseUrl(new URL(this.extSearchUrl)) + .build(ExternalItemSearchClient.class) + .allMethodInfo() + .onFailure().invoke(t -> log.warn("Failed to load external item search methods", t)) + .onFailure().recoverWithNull(); + } catch (MalformedURLException e) { + log.warn("Invalid external item search URL: {}", this.extSearchUrl, e); + } + } this.ensureSearchDefaults(search); return this.getUni( @@ - "extSearchMethods", extSearchEnabled ? - QuarkusRestClientBuilder.newBuilder() - .baseUrl(new URL(this.extSearchUrl)) - .build(ExternalItemSearchClient.class).allMethodInfo() - : Uni.createFrom().nullItem() + "extSearchMethods", extSearchMethods ) ); }software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/upcItemDb/UpcItemDbLookupClient.java (1)
24-29:⚠️ Potential issue | 🟠 MajorHeave to and secure those credentials—stow the headers out of the cache, mate!
The
getFromUpcCodemethod be cavin' in by stowin' those credential headers right into its cache key alongside the barcode. That means each user with a differentuser_keyorkey_typewill sail right past the cache, splittin' the cache hits into pieces and spreadin' credentials where they've no business bein'—in logs, metrics, and cache inspections where any scallywag might spot 'em.Use the
@CacheKeyannotation to tether the cache to the barcode alone, keepin' those secret-bearin' headers off the manifest.⚓ The fix
+import io.quarkus.cache.CacheKey; import io.quarkus.cache.CacheResult; @@ Uni<ObjectNode> getFromUpcCode( `@HeaderParam`("user_key") String key, `@HeaderParam`("key_type") String keyType, - Request barcode + `@CacheKey` Request barcode );
🟠 Major comments (15)
software/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/obj/ObjViewUtils.js-36-43 (1)
36-43:⚠️ Potential issue | 🟠 MajorGuard
valtype before truncation, or the UI may go overboard.Line [36] assumes
valhas.length; ifvalisnull, a number, or another non-string, this can throw and break attribute rendering.🧩 Proposed fix
- if(val.length > 50){ + const safeVal = val == null ? "" : String(val); + if(safeVal.length > 50){ let abbr = $('<abbr></abbr>'); - abbr.text(val.slice(0, 47) + '...'); - abbr.attr("title", val); + abbr.text(safeVal.slice(0, 47) + '...'); + abbr.attr("title", safeVal); output.find(".attVal").append(abbr); } else { - output.find(".attVal").text(val); + output.find(".attVal").text(safeVal); }software/plugins/external-item-search/dev/services/mappings/datakick/datakick_barcode_not_found.json-3-3 (1)
3-3:⚠️ Potential issue | 🟠 MajorBelay that regex, matey—yer pattern be takin' on water!
This here
/datakick/api/items/(?!00888109010058(?=/|$))$be a faulty chart. The$anchor sits right after the lookahead with naught followin' behind it, meanin' it'll only match/datakick/api/items/bare as a ship's hull—no cargo (barcode) aboard. Any real lookup with a barcode like/datakick/api/items/0123456789012won't catch on this stub, and yer WireMock mock'll miss its moorin's.Hoist the corrected pattern to keep the vessel shipshape:
🛠️ Set yer course true
- "urlPathPattern": "/datakick/api/items/(?!00888109010058(?=/|$))$", + "urlPathPattern": "/datakick/api/items/(?!00888109010058$)[^/]+$",The fixed pattern requires at least one character for the barcode, refuses only that cursed
00888109010058, and lets all other barcodes through proper-like.software/plugins/external-item-search/installerSrc/30-plugin-ext_item_search.json-9-23 (1)
9-23:⚠️ Potential issue | 🟠 MajorKeyed providers be enabled with placeholder keys—this invites 401 storms.
On Lines 10-23, turning on key-required providers by default with
" "API keys is likely to cause immediate auth failures. Safer default is disabled + empty key until configured.⚓ Proposed fix
"barcodelookup": { - "enabled": "true", - "apiKey": " " + "enabled": "false", + "apiKey": "" }, @@ "rebrickable": { - "enabled": "true", - "apiKey": " " + "enabled": "false", + "apiKey": "" }, "upcitemdb": { - "enabled": "true", - "apiKey": " " + "enabled": "false", + "apiKey": "" }software/core/oqm-core-base-station/src/main/resources/templates/tags/inputs/barcodeInput.html-3-3 (1)
3-3:⚠️ Potential issue | 🟠 MajorDefault
numberinput can scuttle valid barcodes.On Line 3, defaulting to
type="number"risks stripping leading zeros (common in UPC/EAN), which can cause failed matches. Use text semantics by default, with numeric keypad hints.⚓ Proposed fix
- <input type="{`#if` type??}{type}{`#else`}number{/if}" class="form-control" placeholder="UPC, ISBN..." id="{id}" name="{`#if` name??}{name}{/if}" aria-describedby="{id}Help" {`#if` required??}{required}{/if}> + <input type="{`#if` type??}{type}{`#else`}text{/if}" inputmode="numeric" pattern="[0-9Xx-]*" class="form-control" placeholder="UPC, ISBN..." id="{id}" name="{`#if` name??}{name}{/if}" aria-describedby="{id}Help" {`#if` required??}{required}{/if}>software/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_part_by_num.json-3-23 (1)
3-23:⚠️ Potential issue | 🟠 MajorStub be hard-anchored to one part number, matey.
Line 3 only matches
012345, so other part lookups won’t hit this mapping in dev, even though Line 22 is templated for dynamic output.⚓ Proposed fix
- "urlPathPattern": "/rebrickable/api/v3/lego/parts/012345/", + "urlPathPattern": "/rebrickable/api/v3/lego/parts/[^/]+/",software/core/oqm-core-base-station/src/main/resources/templates/tags/providerInfoCard.html-4-6 (1)
4-6:⚠️ Potential issue | 🟠 MajorAdd
relon external blank-target link, matey.Line 4 uses
target="_blank"withoutrel="noopener noreferrer", which leaves the hatch open to tabnabbing.⚓ Proposed fix
- <a href="{providerInfo.get("homepage").asText()}" target="_blank"> + <a href="{providerInfo.get("homepage").asText()}" target="_blank" rel="noopener noreferrer">software/plugins/external-item-search/src/main/resources/application.yml-34-37 (1)
34-37:⚠️ Potential issue | 🟠 MajorAye, mate—full loggin' be exposin' the treasure chest to prying eyes, savvy?
Lines 35 and 96 set
scope: allon the REST logger. With those API keys bein' sent as query parameters in the BarcodeLookupClient (lines 19 and 24 o' that Java file), full REST logging will haul yer secrets right into the logbooks for all to see.Reduce the log scope or strip the sensitive params before lettin' that logger loose, or ye'll find yer ship plundered.
Also applies to lines 95–98.
.github/workflows/plugin-extItemSearch.yml-16-17 (1)
16-17:⚠️ Potential issue | 🟠 MajorBatten down the hatches—pin that reusable workflow to a full commit SHA.
That
@mainreference be a loose cannon, mate. When ye sail on a moving branch like that, the CI be liable to shift beneath yer feet without so much as a warning. The crew what maintains that upstream workflow could push new code straight into the main line, and yer ship'd change course with it—accidental or otherwise.Lash yerself to a proper commit SHA (a full one, mind ye—not a tag that can be force-pushed). That'll keep yer hull watertight and yer runs reproducible, no matter what winds blow from upstream. Aye, it takes a bit more work to keep the rope taut, but that be the price of keepin' a proper vessel.
software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/rest/passthrough/plugins/externalItemSearch/ItemLookupRestInterface.java-40-40 (1)
40-40:⚠️ Potential issue | 🟠 MajorHoist
@Validonto that@BeanParam, or the validation won't set sail.The
qfield constraints onItemLookupRequestwon't fire without@Validcascading the validation—blank searches can slip through the bilge and sink the ship downstream.🧭 Proposed fix
+import jakarta.validation.Valid; @@ - public Uni<Response> search(`@BeanParam` ItemLookupRequest request) { + public Uni<Response> search(`@Valid` `@BeanParam` ItemLookupRequest request) {Also applies to: 64-68
software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/rebrickable/RebrickableLookupClient.java-21-40 (1)
21-40:⚠️ Potential issue | 🟠 MajorHoist the
@CacheKeyflag—keep credentials off the manifest.Listen well, matey! With this code's current rigging, the cache key be built from every scrap of cargo on the deck—meaning yer API credentials be logged in the cache holdings like a ship's manifest written in the bilge. Every time a new captain's letter of passage arrives, the whole cache busts asunder despite the cargo being the same.
Mark only the search parameter (path or query value) as the true cache key, keeping the Authorization header off the books. Quarkus provides the
@CacheKeyannotation for exactly this purpose—use it to chart which parameters are worth remembering, and leave the credentials to the wind.Suggested course correction
+import io.quarkus.cache.CacheKey; import io.quarkus.cache.CacheResult; @@ - Uni<ObjectNode> partFromNum(`@HeaderParam`("Authorization") String apiKey, `@PathParam`("partNo") String partNumber); + Uni<ObjectNode> partFromNum(`@HeaderParam`("Authorization") String apiKey, `@CacheKey` `@PathParam`("partNo") String partNumber); @@ - Uni<ObjectNode> partsSearch(`@HeaderParam`("Authorization") String apiKey, `@QueryParam`("search") String query); + Uni<ObjectNode> partsSearch(`@HeaderParam`("Authorization") String apiKey, `@CacheKey` `@QueryParam`("search") String query); @@ - Uni<ObjectNode> setFromNum(`@HeaderParam`("Authorization") String apiKey, `@PathParam`("setNo") String setNumber); + Uni<ObjectNode> setFromNum(`@HeaderParam`("Authorization") String apiKey, `@CacheKey` `@PathParam`("setNo") String setNumber); @@ - Uni<ObjectNode> setsSearch(`@HeaderParam`("Authorization") String apiKey, `@QueryParam`("search") String query); + Uni<ObjectNode> setsSearch(`@HeaderParam`("Authorization") String apiKey, `@CacheKey` `@QueryParam`("search") String query);software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/ItemSearchService.java-77-78 (1)
77-78:⚠️ Potential issue | 🟠 MajorEmpty method filters currently search nothing.
The ternary is flipped. When
lookupMethodis empty,methodsbecomes that same empty list, so the nested loop never runs and the “all methods” fallback is lost.⚓ Proposed fix
- Collection<LookupMethod> methods = lookupMethod.isEmpty() ? lookupMethod : - lookupMethod.stream().filter(this.getService().supportedMethods::contains).toList(); + Collection<LookupMethod> methods = lookupMethod.isEmpty() ? + this.getService().supportedMethods : + lookupMethod.stream().filter(this.getService().supportedMethods::contains).toList();software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/upcItemDb/UpcItemDbService.java-108-119 (1)
108-119:⚠️ Potential issue | 🟠 MajorGuard partial offers before dereferencing
merchant.
curOffer.get("merchant").asText()will throw when the upstream payload omits that field. One malformed offer currently sinks the whole provider response instead of just skipping the bad record.⚓ Proposed fix
case "offers": for (JsonNode curOffer : curFieldVal) { - String name = curOffer.get("merchant").asText(); + JsonNode merchant = curOffer.get("merchant"); + if (ResultMappingUtils.isFieldEmpty(merchant)) { + continue; + } + String name = merchant.asText(); if(!ResultMappingUtils.isFieldEmpty(curOffer.get("link"))){ links.put(name, curOffer.get("link").asText()); }software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/barcodeLookup/BarcodeLookupService.java-112-134 (1)
112-134:⚠️ Potential issue | 🟠 MajorHarden this upstream parsing before a bad payload sinks the search.
barcode_formatsassumes exactly two space-separated tokens, and thestoresblock dereferencesname/linkunguarded. One malformed provider record can currently throw and turn the whole lookup into an error result.⚓ Proposed fix
case "barcode_formats": - for(String curBarcode : curFieldVal.asText().split(", ")){ - String[] barcodeParts = curBarcode.split(" "); - identifiers.put(barcodeParts[0], barcodeParts[1]); + for(String curBarcode : curFieldVal.asText().split(",")){ + String[] barcodeParts = curBarcode.trim().split("\\s+", 2); + if (barcodeParts.length < 2) { + continue; + } + identifiers.put(barcodeParts[0], barcodeParts[1]); } break; @@ case "stores": curFieldVal.valueStream() .forEach(curStoreJson -> { - links.put(curStoreJson.get("name").asText(), curStoreJson.get("link").asText()); + JsonNode name = curStoreJson.get("name"); + JsonNode link = curStoreJson.get("link"); + if (ResultMappingUtils.isFieldEmpty(name) || ResultMappingUtils.isFieldEmpty(link)) { + return; + } + links.put(name.asText(), link.asText()); }); break;software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/interfaces/ItemLookupRestInterface.java-81-83 (1)
81-83:⚠️ Potential issue | 🟠 MajorReef this debug log before it spills user input into the logbook.
searchcarries raw lookup terms and URLs. Logging the whole bean leaks request data into debug logs for little gain. Log only high-level metadata or a redacted term instead.⚓ Proposed fix
- log.debug("Search parameters: {}", search); + log.debug( + "External item search request: services={}, sources={}, methods={}, keepNotFound={}", + search.getServices(), + search.getSources(), + search.getLookupMethods(), + search.isKeepNotFound() + );software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/ItemSearchService.java-43-48 (1)
43-48:⚠️ Potential issue | 🟠 MajorHoist this code back to port, matey—there be two rocks ahead.
First: Ye be callin'
isEnabled()from the base constructor, which be a treacherous course. That vessel be dispatching into subclass overrides whilst the ship ain't fully rigged yet. Look atBarcodeLookupService.isEnabled()—it depends on theapiKeybein' set fast. If ye call that during construction, theproviderInfo.enabledgets baked asfalsebefore the subclass finishes lashing the lines. Use theenabledparameter passed straight to the constructor instead:- .enabled(this.isEnabled()) + .enabled(enabled)Second: The filter logic for multi-search be backwards as a ship in a hurricane. At line 77-78:
Collection<LookupMethod> methods = lookupMethod.isEmpty() ? lookupMethod : lookupMethod.stream().filter(this.getService().supportedMethods::contains).toList();That be signin' on an empty crew when none be requested! Compare it to line 79, which be doin' it proper:
Collection<LookupSource> sources = source.isEmpty() ? this.getService().supportedSources : source;When the method list be empty, ye should use all supported methods as the default. Fix it thus:
-Collection<LookupMethod> methods = lookupMethod.isEmpty() ? lookupMethod : +Collection<LookupMethod> methods = lookupMethod.isEmpty() ? this.getService().supportedMethods :
🟡 Minor comments (12)
docs/development/developmentFlow.md-22-22 (1)
22-22:⚠️ Potential issue | 🟡 MinorAvast! Mind yer capitalization, matey!
The word "github" should be properly capitalized as "GitHub" to match the official brand name, sailor. Keep the ship's log tidy and professional-like!
✍️ Proposed fix fer the capitalization
-All issues are to be assigned to the OQM github project for tracking purposes. We use the features of the project interface to track issues, assign them to team members, and manage workflows. +All issues are to be assigned to the OQM GitHub project for tracking purposes. We use the features of the project interface to track issues, assign them to team members, and manage workflows.software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/utils/ResultMappingUtils.java-8-20 (1)
8-20:⚠️ Potential issue | 🟡 MinorHoist the Mainsail! Keep
MissingNodefrom Slipping Through the RiggingAye, there be a leak in the hull, matey. Yer code checks fer null and JSON nulls at line 8, but that scallywag
MissingNodeslips right past like a rat in the dark and sails on through to line 20 returnin'false. That be foul business—absent fields get marked as full cargo when they ought to be empty.Every one of yer call sites be usin'
.get()on them JsonNodes (see UpcItemDbService lines 113, 117 and the like), and as any salty dog knows,.get("key")returnsMissingNodewhen the key be missin'. Ye need to tighten this knot:⚓ Proposed patch
- if (curFieldVal == null || curFieldVal.isNull()) { + if (curFieldVal == null || curFieldVal.isNull() || curFieldVal.isMissingNode()) { return true; }The
isMissingNode()method be already sailin' in yer codebase (spotted in JacksonObjectNodeBodyHandler), so this fix'll keep yer decks shipshape and all fields accounted fer proper-like.software/plugins/external-item-search/dev/services/mappings/upcitemdb/README.md-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorTrim the empty bullet from the README.
Line [3] leaves a blank list item, matey, which makes the doc look unfinished.
🧹 Proposed fix
- -software/plugins/external-item-search/dev/services/mappings/README.md-3-9 (1)
3-9:⚠️ Potential issue | 🟡 MinorPatch these typos, sailor—docs be leaking polish.
Line 3 and Line 9 contain spelling/casing errors that hurt readability.
🛠️ Suggested typo fixes
-The json documents in this directory outline the responses given from the various data providers we support. They are wiremock stibs that simulate that bhavior. +The JSON documents in this directory outline the responses from the various data providers we support. They are WireMock stubs that simulate that behavior. @@ - - `devKeyBad` is always used as an INvalid API key, in turn + - `devKeyBad` is always used as an invalid API key, in turnsoftware/plugins/external-item-search/README.md-7-9 (1)
7-9:⚠️ Potential issue | 🟡 MinorTighten this README wording, matey.
Lines 7-9 read awkwardly, and “API's” should be “APIs”.
🛠️ Suggested wording cleanup
-An aggregation service to amalgamate item data from multiple sources into a single format for use in OQM. - -Caches results from upstream API's to reduce usage where unnecessary. +An aggregation service that combines item data from multiple sources into a single OQM format. + +Caches results from upstream APIs to reduce unnecessary usage.software/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_parts_search.json-662-687 (1)
662-687:⚠️ Potential issue | 🟡 MinorFix corrupted character in fixture text values.
Line 662 and Line 686 contain
45�, which is mojibake and will render incorrectly in UI/tests; use45°instead.⚓ Proposed fix
- "name": "Wedge Sloped 45� 2 x 2 Corner", + "name": "Wedge Sloped 45° 2 x 2 Corner", ... - "name": "Wedge Sloped 45� 2 x 2 Corner with White Dots, Circles print", + "name": "Wedge Sloped 45° 2 x 2 Corner with White Dots, Circles print",software/core/oqm-core-base-station/src/main/resources/templates/tags/itemAddEditModal.html-36-36 (1)
36-36:⚠️ Potential issue | 🟡 MinorArr, there be a typo in yer label, sailor!
"Search Metod" be missin' an 'h' — should read "Search Method" for proper display to the crew.
✏️ Proposed fix
- <label for="addEditProductSearchMethodInput" class="form-label">Search Metod</label> + <label for="addEditProductSearchMethodInput" class="form-label">Search Method</label>software/core/oqm-core-base-station/src/main/resources/templates/tags/itemAddEditModal.html-45-47 (1)
45-47:⚠️ Potential issue | 🟡 MinorBelay that! Duplicate ID be causin' trouble in these waters!
The
id="addEditProductSearchTermInput"be used twice — once on the input (line 46 via the barcodeInput tag) and again on the form-text div (line 47). This'll confuse the DOM lookups and break accessibility, it will. The static analysis lookout spotted this as well.🔧 Proposed fix to give the div a unique identifier
<div class="mb-3"> <label for="addEditProductSearchTermInput" class="form-label">Search Term</label> {`#inputs/barcodeInput` id='addEditProductSearchTermInput' name='q' type='text' searchSubmit=true required=true /} - <div id="addEditProductSearchTermInput" class="form-text">Scan in or enter a search term.</div> + <div id="addEditProductSearchTermInputHelp" class="form-text">Scan in or enter a search term.</div> </div>software/core/oqm-core-base-station/src/main/resources/templates/tags/itemAddEditModal.html-83-85 (1)
83-85:⚠️ Potential issue | 🟡 MinorAvast! "foo" be placeholder cargo that shouldn't ship, matey!
The Errors tab content shows "foo" instead of proper error displayin' machinery. This be needin' a proper error list or message container before the vessel sets sail.
🛠️ Proposed fix to display errors properly
<div class="tab-pane fade" id="extItemSearchSearchResultsTabErrorsTabContent" role="tabpanel" aria-labelledby="extItemSearchSearchResultsTabErrorsTab" tabindex="0"> - foo + <div id="extItemSearchSearchResultsTableErrors"> + <!-- Error messages will be populated here --> + </div> </div>software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/model/lookupResult/ExtItemLookupErrResult.java-35-40 (1)
35-40:⚠️ Potential issue | 🟡 MinorHoist a fallback fer missing error text.
If
errMessageis null/blank, Line 39 can render an unhelpful"null"message to crew on deck.⚓ Proposed fix
public String getDisplayMessage() { - return (this.getErrCode() == null? "" : this.getErrCode() + " - ") + this.errMessage; + String msg = (this.errMessage == null || this.errMessage.isBlank()) ? "Unknown external lookup error" : this.errMessage; + return (this.getErrCode() == null ? "" : this.getErrCode() + " - ") + msg; }software/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/item/ExtItemSearch.js-205-207 (1)
205-207:⚠️ Potential issue | 🟡 MinorAvast! This handler be an empty hull, leavin' "no results" adrift at sea!
When a
NO_RESULTStype comes through the riggin' at line 485, it calls this here function which does nothin' at all. The user won't be informed that a particular service returned no matches - it'll just vanish like a ship in the fog.Consider at least loggin' or displayin' somethin' to the crew so they know what happened.
🔧 Proposed fix to at least log the no-results
static async handleNotFoundResult(result) { - //TODO + console.debug("No results from service:", result.service); + // Optionally display in UI or increment a "no results" counter }software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/rebrickable/RebrickableService.java-270-288 (1)
270-288:⚠️ Potential issue | 🟡 MinorAye, this 404 handlin' be serviceable, but tied to a specific phrase!
The error handlin' at line 276 compares against a hardcoded string
"No Part matches the given query."- if the Rebrickable API ever changes their error messages, this logic will fail silently and ye'll get generic errors instead of proper "no results" responses.Consider loggin' the actual detail message when it doesn't match, so ye'll have a record in the ship's log if things change.
🛡️ Suggested defensive improvement
if (errorDeets.get("detail").asText().equals("No Part matches the given query.")) { return Optional.of( this.setupResponseBuilder(LookupResultNoResults.builder(), source, method) .detail("No Part matches the given query.") .build() ); +} else { + log.debug("Unrecognized 404 detail message from Rebrickable: {}", errorDeets.get("detail").asText()); }
🧹 Nitpick comments (11)
software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/exceptions/SearchNotSupportedException.java (1)
5-10: Consider stowing the search type in yer hold, matey!The exception be seaworthy as she stands, but consider these refinements for smoother sailing:
serialVersionUID: Since Exception be Serializable, 'tis wise seamanship to declare an explicit
serialVersionUIDto avoid surprises when yer cargo crosses different ports (JVM versions).Store the SearchType: Ye be usin' the
SearchTypeonly fer the message, but ye ain't keepin' it as a field. If downstream code needs to inspect which search type caused the trouble programmatically, they'll be left high and dry.⚓ Proposed refactor to add serialVersionUID and store SearchType
public class SearchNotSupportedException extends Exception { - + private static final long serialVersionUID = 1L; + private final SearchType type; + public SearchNotSupportedException(SearchType type) { super("Not supported: " + type); + this.type = type; } + + public SearchType getType() { + return type; + } }software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/utils/ResultMappingUtils.java (1)
5-6: Batten down the hatches with a proper private constructor, savvy?This utility class be standing as a
finalbeacon, but she's got no privatedecking to prevent unwanted boarders from spawning new instances. Even though no scallywag be instantiating her at present, best maritime law says a static-only helper ship should bar the gangway proper-like.⚓ Proposed patch
public final class ResultMappingUtils { + private ResultMappingUtils() { + throw new IllegalStateException("Utility class"); + } public static boolean isFieldEmpty(JsonNode curFieldVal) {software/plugins/external-item-search/dev/services/mappings/upcitemdb/upcitemdb_trial_barcode.json (1)
16-20: Trim the stub latency, matey, lest local runs drag anchor.A 5s median delay is hefty for a default dev mapping and can slow test/dev feedback loops when this endpoint is hit repeatedly. Consider a smaller default and keep long-latency scenarios in dedicated fixtures.
⚓ Proposed tweak
"delayDistribution": { "type": "lognormal", - "median": 5000, + "median": 500, "sigma": 0.4 },software/plugins/external-item-search/dev/services/mappings/rebrickable/README.md (1)
1-4: Add one more chart note fer the crew: auth expectations.Consider adding a short line about required auth/header format used by your mappings so future maintainers can wire new stubs faster.
software/plugins/external-item-search/src/test/resources/application.yaml (1)
16-17: Trim the emptyquarkus.httpblock after port removal.Aye, once Line 17 is commented,
http:is left hollow and can confuse future config reads; better strike both lines cleanly.⚓ Suggested cleanup
quarkus: - http: -# port: 8081 log: level: DEBUGsoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/dataKick/DataKickLookupClient.java (1)
3-15: Avast! There be unused cargo cluttering the hold, matey.These imports be weighin' down the manifest without pullin' their weight:
JsonNode(line 3)ObjectNode(line 5)Multi(line 8)CompletionStage(line 15)Best toss 'em overboard to keep the deck tidy, says I.
🧹 Proposed cleanup to lighten the load
package tech.ebp.oqm.plugin.extItemSearch.service.extItemSearchService.providers.dataKick; -import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.ObjectNode; import io.opentelemetry.instrumentation.annotations.WithSpan; import io.quarkus.cache.CacheResult; -import io.smallrye.mutiny.Multi; import io.smallrye.mutiny.Uni; import jakarta.ws.rs.GET; import jakarta.ws.rs.Path; import jakarta.ws.rs.PathParam; import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; -import java.util.concurrent.CompletionStage; -software/plugins/external-item-search/dev/services/mappings/datakick/datakick_barcode.json (1)
11-15: Avast! That be a mighty long delay for dev waters, matey.A median of 7000ms (7 seconds) be quite the wait when testin' in port. Most other stubs in this fleet be usin' 100-500ms delays. If ye intend to simulate a sluggish external service, then aye, carry on — but if not, ye might want to trim that sail.
⏱️ Proposed fix to speed up the voyage
"delayDistribution": { "type": "lognormal", - "median": 7000, + "median": 500, "sigma": 0.4 },software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/OverviewUi.java (2)
115-119: Arr, ye be buildin' a new ship every time ye need to fetch cargo!Creatin' a new
ExternalItemSearchClientviaQuarkusRestClientBuilderon every request be wasteful, like buildin' a new dinghy each time ye row to shore. This be addin' overhead and potentially causin' resource leaks if the client holds connections.Consider hoistin' the client creation to a
@PostConstructmethod or injectin' a lazily-initialized singleton, so ye reuse the same vessel for all voyages.⚓ Proposed approach to cache the client
// In class fields: private ExternalItemSearchClient extSearchClient; `@PostConstruct` void initExtSearchClient() throws MalformedURLException { if (!this.extSearchUrl.isBlank()) { this.extSearchClient = QuarkusRestClientBuilder.newBuilder() .baseUrl(new URL(this.extSearchUrl)) .build(ExternalItemSearchClient.class); } } // Then in overview(): "extSearchMethods", extSearchEnabled ? this.extSearchClient.allMethodInfo() : Uni.createFrom().nullItem()Note: Since
OverviewUiis@RequestScoped, ye may want to move client creation to an@ApplicationScopedprovider bean instead.
70-71: A single space as default value be a peculiar flag, matey.Usin'
defaultValue = " "(a space) and then checkin'isBlank()works, but it be a mite confusin'. An empty string""or a more explicit sentinel might be clearer to the next sailor readin' the charts.software/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/item/ExtItemSearch.js (1)
36-46: Arr, these display functions be needin' some cargo to haul!These here helper functions be marked with TODO comments and presently just return what they're given without transformation. If ye intend to ship this vessel, best be makin' a note to circle back and fill these holds proper-like.
Would ye have me open an issue to track completin' these display transformation functions, or shall they remain as-is for now?
software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/rebrickable/RebrickableService.java (1)
85-148: These two methods be haulin' nearly the same cargo, matey.
partJsonToResultandsetJsonToResultshare a great deal of similar structure - both build the same collections, both set the brand attribute, and both iterate over JSON properties with similar switch logic. The main differences be the field names (part_numvsset_num, etc.).If ye find yerself maintainin' both in the future, consider extractin' the common logic into a shared helper, perhaps passin' in the field name mappings as parameters.
Also applies to: 169-215
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ad801148-4d11-41e2-8d5e-0bc513174f48
📒 Files selected for processing (114)
.github/dependabot.yml.github/workflows/plugin-extItemSearch.ymldeployment/Single Host/Station-Captain/src/lib/ConfigManager.pydeployment/Single Host/Station-Captain/src/oqm-config.pydocs/development/developmentFlow.mdsoftware/core/oqm-core-api/build.gradlesoftware/core/oqm-core-api/gradle.propertiessoftware/core/oqm-core-base-station/build.gradlesoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/rest/passthrough/plugins/externalItemSearch/ItemLookupRestInterface.javasoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/HelpUi.javasoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/InventoryItemUi.javasoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/OverviewUi.javasoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/service/ExternalItemSearchClient.javasoftware/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/AssociatedLinks.jssoftware/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/Identifiers.jssoftware/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/item/ExtItemSearch.jssoftware/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/obj/ObjViewUtils.jssoftware/core/oqm-core-base-station/src/main/resources/templates/tags/inputs/barcodeInput.htmlsoftware/core/oqm-core-base-station/src/main/resources/templates/tags/itemAddEditModal.htmlsoftware/core/oqm-core-base-station/src/main/resources/templates/tags/providerInfoCard.htmlsoftware/core/oqm-core-base-station/src/main/resources/templates/webui/pages/help.htmlsoftware/core/oqm-core-base-station/src/main/resources/templates/webui/pages/items.htmlsoftware/core/oqm-core-base-station/src/main/resources/templates/webui/pages/overview.htmlsoftware/libs/core-api-lib-quarkus/deployment/pom.xmlsoftware/libs/core-api-lib-quarkus/integration-tests/pom.xmlsoftware/libs/core-api-lib-quarkus/pom.xmlsoftware/libs/core-api-lib-quarkus/runtime/pom.xmlsoftware/libs/core-api-lib-quarkus/runtime/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/runtime/restClient/searchObjects/FileSearchObject.javasoftware/libs/core-api-lib-quarkus/runtime/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/runtime/restClient/searchObjects/ImageSearch.javasoftware/plugins/external-item-search/README.mdsoftware/plugins/external-item-search/build.gradlesoftware/plugins/external-item-search/dev/services/mappings/README.mdsoftware/plugins/external-item-search/dev/services/mappings/barcodelookup/README.mdsoftware/plugins/external-item-search/dev/services/mappings/barcodelookup/barcodelookup_barcode.jsonsoftware/plugins/external-item-search/dev/services/mappings/barcodelookup/barcodelookup_barcode_bad_key.jsonsoftware/plugins/external-item-search/dev/services/mappings/barcodelookup/barcodelookup_barcode_no_results.jsonsoftware/plugins/external-item-search/dev/services/mappings/barcodelookup/barcodelookup_query.jsonsoftware/plugins/external-item-search/dev/services/mappings/barcodelookup/barcodelookup_query_bad_key.jsonsoftware/plugins/external-item-search/dev/services/mappings/barcodelookup/barcodelookup_query_not_found.jsonsoftware/plugins/external-item-search/dev/services/mappings/datakick/README.mdsoftware/plugins/external-item-search/dev/services/mappings/datakick/datakick_barcode.jsonsoftware/plugins/external-item-search/dev/services/mappings/datakick/datakick_barcode_not_found.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/README.mdsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_part_by_num.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_part_by_num_bad_key.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_part_by_num_not_found.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_parts_search.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_parts_search_bad_key.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_parts_search_not_found.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_set_by_num.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_set_by_num_bad_key.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_set_by_num_not_found.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_sets_search.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_sets_search_bad_key.jsonsoftware/plugins/external-item-search/dev/services/mappings/rebrickable/rebrickable_sets_search_not_found.jsonsoftware/plugins/external-item-search/dev/services/mappings/upcitemdb/README.mdsoftware/plugins/external-item-search/dev/services/mappings/upcitemdb/upcitemdb_barcode.jsonsoftware/plugins/external-item-search/dev/services/mappings/upcitemdb/upcitemdb_trial_barcode.jsonsoftware/plugins/external-item-search/dev/wmMapping/mappings/barcodelookup_barcode.jsonsoftware/plugins/external-item-search/dev/wmMapping/mappings/datakick_barcode.jsonsoftware/plugins/external-item-search/dev/wmMapping/mappings/rebrickable_part.jsonsoftware/plugins/external-item-search/dev/wmMapping/mappings/upcitemdb_barcode.jsonsoftware/plugins/external-item-search/dev/wmMapping/mappings/upcitemdb_trial_barcode.jsonsoftware/plugins/external-item-search/gradle.propertiessoftware/plugins/external-item-search/installerSrc/30-plugin-ext_item_search.jsonsoftware/plugins/external-item-search/installerSrc/plugin-ext_item_search-config.listsoftware/plugins/external-item-search/src/integrationTest/java/tech/ebp/oqm/plugin/extItemSearch/BootTestIT.javasoftware/plugins/external-item-search/src/main/docker/Dockerfile.jvmsoftware/plugins/external-item-search/src/main/docker/Dockerfile.legacy-jarsoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/interfaces/ItemLookupRestInterface.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/lifecycle/LifecycleBean.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/model/ExtItemLookupProviderInfo.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/model/ExtItemSearch.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/model/SearchType.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/model/lookupResult/ExtItemLookupErrResult.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/model/lookupResult/ExtItemLookupResult.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/model/lookupResult/LookupResult.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/model/lookupResult/LookupResultNoResults.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/model/lookupResult/ResultType.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/ExtItemLookupService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/ItemSearchService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/exceptions/SearchNotSupportedException.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/barcodeLookup/BarcodeLookupClient.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/barcodeLookup/BarcodeLookupService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/dataKick/DataKickLookupClient.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/dataKick/DatakickService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/rebrickable/RebrickableLookupClient.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/rebrickable/RebrickableService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/upcItemDb/UpcItemDbLookupClient.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/providers/upcItemDb/UpcItemDbService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/utils/LookupMethod.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/utils/LookupService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/utils/LookupSource.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/extItemSearchService/utils/ResultMappingUtils.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/ItemSearchService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/ItemApiSearchService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/lego/LegoLookupService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/lego/rebrickable/RebrickableLookupClient.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/lego/rebrickable/RebrickableService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/ApiProductSearchService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/barcodeLookup/BarcodeLookupClient.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/barcodeLookup/BarcodeLookupService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/dataKick/DataKickLookupClient.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/dataKick/DataKickService.javasoftware/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/upcItemDb/UpcItemDbService.javasoftware/plugins/external-item-search/src/main/resources/application.ymlsoftware/plugins/external-item-search/src/native-test/java/tech/ebp/openQuarterMaster/BootIT.javasoftware/plugins/external-item-search/src/test/java/tech/ebp/oqm/plugin/extItemSearch/BootTest.javasoftware/plugins/external-item-search/src/test/resources/application.yamlsoftware/plugins/external-item-search/temp/webPage/AdafruitWebProductScrapeService.javasoftware/plugins/external-item-search/temp/webPage/AmazonWebProductScrapeService.javasoftware/plugins/external-item-search/temp/webPage/GenericWebProductScrapeService.javasoftware/plugins/external-item-search/temp/webPage/WebPageProductScrapeService.javasoftware/plugins/storagotchi/build.gradle
💤 Files with no reviewable changes (17)
- software/plugins/external-item-search/dev/wmMapping/mappings/upcitemdb_barcode.json
- software/plugins/external-item-search/dev/wmMapping/mappings/datakick_barcode.json
- software/plugins/external-item-search/dev/wmMapping/mappings/rebrickable_part.json
- software/plugins/external-item-search/dev/wmMapping/mappings/upcitemdb_trial_barcode.json
- software/plugins/external-item-search/dev/wmMapping/mappings/barcodelookup_barcode.json
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/lego/LegoLookupService.java
- software/plugins/external-item-search/src/native-test/java/tech/ebp/openQuarterMaster/BootIT.java
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/dataKick/DataKickLookupClient.java
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/ItemApiSearchService.java
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/lego/rebrickable/RebrickableService.java
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/ItemSearchService.java
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/dataKick/DataKickService.java
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/barcodeLookup/BarcodeLookupClient.java
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/lego/rebrickable/RebrickableLookupClient.java
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/ApiProductSearchService.java
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/barcodeLookup/BarcodeLookupService.java
- software/plugins/external-item-search/src/main/java/tech/ebp/oqm/plugin/extItemSearch/service/searchServices/api/product/upcItemDb/UpcItemDbService.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: CI-Pipeline / Unit Tests
- GitHub Check: CI-Pipeline / Integration Tests
- GitHub Check: CI-Pipeline / Unit Tests
- GitHub Check: CI-Pipeline / Integration Tests
🧰 Additional context used
🪛 HTMLHint (1.9.2)
software/core/oqm-core-base-station/src/main/resources/templates/tags/itemAddEditModal.html
[error] 39-39: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 39-39: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 39-39: Tag must be paired, no start tag: [ ]
(tag-pair)
[error] 47-47: The id value [ addEditProductSearchTermInput ] must be unique.
(id-unique)
software/core/oqm-core-base-station/src/main/resources/templates/tags/inputs/barcodeInput.html
[error] 3-3: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 3-3: Special characters must be escaped : [ > ].
(spec-char-escape)
software/core/oqm-core-base-station/src/main/resources/templates/tags/providerInfoCard.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
[error] 4-4: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 4-4: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 6-6: Tag must be paired, no start tag: [ ]
(tag-pair)
🪛 LanguageTool
software/plugins/external-item-search/README.md
[style] ~5-~5: Consider a more concise word here.
Context: ...ching for items outside the OQM system, in order to get their data into it. An aggregation...
(IN_ORDER_TO_PREMIUM)
software/plugins/external-item-search/dev/services/mappings/README.md
[grammar] ~3-~3: Ensure spelling is correct
Context: ...y are wiremock stibs that simulate that bhavior. ## Conventions - API Keys - devKey i...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
Code Coverage
|
Code Coverage
|
1 similar comment
Code Coverage
|
Code Coverage
|
Code Coverage
|
1 similar comment
Code Coverage
|
Code Coverage
|
1 participant
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores