feat(books): autocomplete for the series/universe/cycle fields (#179)#187
Merged
Conversation
The cycle/universe/series fields on the book form were free-text inputs: a user had to retype the universe name for every book, and a single different character silently created a brand-new universe (or the wrong field got filled). Give the four fields the same Choices.js shape as the author/publisher pickers. Backend - CollaneController::searchApi gains a `field` param (whitelisted) that maps each field to its real suggestion source: - collana -> collane.nome (tipo=serie) + libri.collana - serie_padre -> collane.nome WHERE tipo='universo' (existing universes) - gruppo_serie -> collane.gruppo_serie - ciclo_serie -> collane.ciclo Tables/columns are whitelisted (never from input); the query value and tipo are bound. Frontend - each field becomes a single-select Choices.js mirrored into a hidden input that submits its value unchanged. Typing >=2 letters proposes existing values; the typed text is always offered too (and committed on blur/Enter) so a genuinely new value can still be created. The ISBN scraper fills collana through the exposed setter so the value shows in the control. New i18n key in all 4 locales. Tests: new issue-179 regression spec (suggest existing + create new, all 4 fields); series-cycles + full-test migrated to set these via the autocomplete. Validated: PHPStan L5, issue-179 5/5, series-cycles 15/15.
|
Warning Review limit reached
More reviews will be available in 39 minutes and 3 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughSostituisce i campi testuali serie/collana nel form libro con autocomplete Choices.js (gruppo_serie, serie_padre, collana, ciclo_serie). Il backend ModificheAutocomplete serie/collana (issue
Sequence DiagramsequenceDiagram
actor Utente
participant BookForm as book_form.php (JS)
participant CollaneAPI as /api/collane/search
participant DB as Database
Utente->>BookForm: digita ≥2 caratteri in select[data-series-autocomplete]
BookForm->>CollaneAPI: GET ?field=collana&q=xyz
CollaneAPI->>CollaneAPI: valida field, escaping LIKE, itera sorgenti
CollaneAPI->>DB: SELECT DISTINCT … FROM (UNION …) LIMIT 10
DB-->>CollaneAPI: righe corrispondenti
CollaneAPI-->>BookForm: JSON array di suggerimenti
BookForm->>BookForm: aggiorna opzioni Choices.js (+ opzione "digita nuovo")
Utente->>BookForm: seleziona opzione o preme Enter
BookForm->>BookForm: sincronizza input[hidden] con valore scelto
BookForm->>BookForm: window.__seriesAutocomplete[field] = setter
Stima dello sforzo di revisione🎯 4 (Complex) | ⏱️ ~60 minutes PR potenzialmente correlate
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Views/libri/partials/book_form.php`:
- Around line 511-513: Replace all instances of HtmlHelper::e() with
htmlspecialchars(..., ENT_QUOTES, 'UTF-8') in the book_form.php view file. This
applies to all occurrences of HtmlHelper::e($gruppoSerieVal) in the option value
attribute, option text content, and hidden input value attribute across the
specified line ranges (511-513, 521-523, 540-542, 557-559). According to the
codebase conventions, HtmlHelper::e() should not be used in view files; instead
use the native PHP htmlspecialchars function with the appropriate ENT_QUOTES and
UTF-8 parameters for proper HTML escaping.
- Around line 2229-2237: The _onEnterKey override for the series field does not
handle the case where the user types a value different from the highlighted
dropdown item. The current logic only commits the typed value when there is no
highlighted item, but delegates to origEnter when a highlighted item exists,
which incorrectly selects the wrong suggestion. Extract the text content of the
highlighted item (hl), compare it with the typed value, and if they differ, call
event.preventDefault() and commitTyped(typed) to prevent the wrong suggestion
from being selected. This should mirror the same pattern already correctly
implemented in initializePublishersChoices() for publisher selection, ensuring
the user's typed input is preserved instead of the highlighted dropdown
suggestion when they differ.
In `@tests/full-test.spec.js`:
- Around line 690-699: The collana field configuration in the evaluate block
bypasses the actual Choices.js autocomplete interaction flow by directly calling
the setter or setting the hidden input value, which means the test does not
verify that the autocomplete widget functions correctly. Replace the
page.evaluate approach with actual user interactions: use fill to input the
value into the collana field, add a waitForTimeout to allow suggestions to
appear, and then click on the matching suggestion from the dropdown to properly
simulate user interaction with the Choices.js autocomplete widget, consistent
with how other autocomplete fields are tested elsewhere in the test suite.
In `@tests/issue-179-series-autocomplete.spec.js`:
- Around line 15-20: The test.skip condition at the beginning of the suite only
validates ADMIN_EMAIL but does not validate ADMIN_PASS, which means the test
will run and fail during the login attempt if the password credential is
missing. Extend the skip condition to check that both ADMIN_EMAIL and ADMIN_PASS
environment variables are properly configured before allowing the test to
execute, ensuring complete credential validation before attempting the page.fill
calls for email and password fields.
In `@tests/series-cycles.spec.js`:
- Around line 154-166: The setSeriesAutocomplete function bypasses the actual
Choices.js end-to-end flow by directly injecting values via page.evaluate and
hidden input manipulation, which means tests can pass even if Choices.js
functionality like fetching suggestions, dropdown opening, or selection is
broken. Replace the current implementation with a simulation of the real user
interaction flow: fill the input field with the value, wait briefly for
suggestions to appear, and then click on the matching suggestion. This updated
approach should then be reused consistently wherever setSeriesAutocomplete is
called throughout the test file (in the test cases around lines 171-176, 351,
and 395).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 89656ffb-ccc6-4bc7-8f8b-8f52dc08f83b
📒 Files selected for processing (9)
app/Controllers/CollaneController.phpapp/Views/libri/partials/book_form.phplocale/de_DE.jsonlocale/en_US.jsonlocale/fr_FR.jsonlocale/it_IT.jsontests/full-test.spec.jstests/issue-179-series-autocomplete.spec.jstests/series-cycles.spec.js
Comment on lines
+511
to
+513
| <?php if ($gruppoSerieVal !== ''): ?><option value="<?= HtmlHelper::e($gruppoSerieVal) ?>" selected><?= HtmlHelper::e($gruppoSerieVal) ?></option><?php endif; ?> | ||
| </select> | ||
| <input type="hidden" id="gruppo_serie" name="gruppo_serie" value="<?php echo HtmlHelper::e($gruppoSerieVal); ?>" /> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Sostituisci HtmlHelper::e() nei nuovi campi autocomplete.
Questi valori finiscono in attributi HTML e testo visibile; le nuove righe devono usare direttamente htmlspecialchars(..., ENT_QUOTES, 'UTF-8').
Correzione proposta
- <?php if ($gruppoSerieVal !== ''): ?><option value="<?= HtmlHelper::e($gruppoSerieVal) ?>" selected><?= HtmlHelper::e($gruppoSerieVal) ?></option><?php endif; ?>
+ <?php if ($gruppoSerieVal !== ''): ?><option value="<?= htmlspecialchars($gruppoSerieVal, ENT_QUOTES, 'UTF-8') ?>" selected><?= htmlspecialchars($gruppoSerieVal, ENT_QUOTES, 'UTF-8') ?></option><?php endif; ?>
</select>
- <input type="hidden" id="gruppo_serie" name="gruppo_serie" value="<?php echo HtmlHelper::e($gruppoSerieVal); ?>" />
+ <input type="hidden" id="gruppo_serie" name="gruppo_serie" value="<?php echo htmlspecialchars($gruppoSerieVal, ENT_QUOTES, 'UTF-8'); ?>" />
...
- <?php if ($seriePadreVal !== ''): ?><option value="<?= HtmlHelper::e($seriePadreVal) ?>" selected><?= HtmlHelper::e($seriePadreVal) ?></option><?php endif; ?>
+ <?php if ($seriePadreVal !== ''): ?><option value="<?= htmlspecialchars($seriePadreVal, ENT_QUOTES, 'UTF-8') ?>" selected><?= htmlspecialchars($seriePadreVal, ENT_QUOTES, 'UTF-8') ?></option><?php endif; ?>
</select>
- <input type="hidden" id="serie_padre" name="serie_padre" value="<?php echo HtmlHelper::e($seriePadreVal); ?>" />
+ <input type="hidden" id="serie_padre" name="serie_padre" value="<?php echo htmlspecialchars($seriePadreVal, ENT_QUOTES, 'UTF-8'); ?>" />
...
- <?php if ($collanaVal !== ''): ?><option value="<?= HtmlHelper::e($collanaVal) ?>" selected><?= HtmlHelper::e($collanaVal) ?></option><?php endif; ?>
+ <?php if ($collanaVal !== ''): ?><option value="<?= htmlspecialchars($collanaVal, ENT_QUOTES, 'UTF-8') ?>" selected><?= htmlspecialchars($collanaVal, ENT_QUOTES, 'UTF-8') ?></option><?php endif; ?>
</select>
- <input type="hidden" id="collana" name="collana" value="<?php echo HtmlHelper::e($collanaVal); ?>" />
+ <input type="hidden" id="collana" name="collana" value="<?php echo htmlspecialchars($collanaVal, ENT_QUOTES, 'UTF-8'); ?>" />
...
- <?php if ($cicloSerieVal !== ''): ?><option value="<?= HtmlHelper::e($cicloSerieVal) ?>" selected><?= HtmlHelper::e($cicloSerieVal) ?></option><?php endif; ?>
+ <?php if ($cicloSerieVal !== ''): ?><option value="<?= htmlspecialchars($cicloSerieVal, ENT_QUOTES, 'UTF-8') ?>" selected><?= htmlspecialchars($cicloSerieVal, ENT_QUOTES, 'UTF-8') ?></option><?php endif; ?>
</select>
- <input type="hidden" id="ciclo_serie" name="ciclo_serie" value="<?php echo HtmlHelper::e($cicloSerieVal); ?>" />
+ <input type="hidden" id="ciclo_serie" name="ciclo_serie" value="<?php echo htmlspecialchars($cicloSerieVal, ENT_QUOTES, 'UTF-8'); ?>" />As per path instructions, Mai usare HtmlHelper::e() nelle view — usare htmlspecialchars(..., ENT_QUOTES, 'UTF-8').
Also applies to: 521-523, 540-542, 557-559
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/Views/libri/partials/book_form.php` around lines 511 - 513, Replace all
instances of HtmlHelper::e() with htmlspecialchars(..., ENT_QUOTES, 'UTF-8') in
the book_form.php view file. This applies to all occurrences of
HtmlHelper::e($gruppoSerieVal) in the option value attribute, option text
content, and hidden input value attribute across the specified line ranges
(511-513, 521-523, 540-542, 557-559). According to the codebase conventions,
HtmlHelper::e() should not be used in view files; instead use the native PHP
htmlspecialchars function with the appropriate ENT_QUOTES and UTF-8 parameters
for proper HTML escaping.
Source: Path instructions
Comment on lines
+2229
to
+2237
| if (typeof choice._onEnterKey === 'function') { | ||
| const origEnter = choice._onEnterKey.bind(choice); | ||
| choice._onEnterKey = function (event, hasActiveDropdown) { | ||
| const typed = input ? input.value.trim() : ''; | ||
| const dd = wrapper ? wrapper.querySelector('.choices__list--dropdown') : null; | ||
| const hl = dd ? dd.querySelector('.choices__item--selectable.is-highlighted') : null; | ||
| if (typed && !hl) { event.preventDefault(); commitTyped(typed); return; } | ||
| return origEnter(event, hasActiveDropdown); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, verify file exists and check its structure
if [ -f "app/Views/libri/partials/book_form.php" ]; then
wc -l "app/Views/libri/partials/book_form.php"
echo "---"
# Get lines around 2229-2237 to see the actual code
sed -n '2200,2250p' "app/Views/libri/partials/book_form.php"
else
echo "File not found"
find . -name "book_form.php" -type f
fiRepository: fabiodalez-dev/Pinakes
Length of output: 3063
🏁 Script executed:
# Search for Choices library usage and DOM structure references
rg "choices__item" "app/Views" -A 2 -B 2
echo "---"
# Search for any other references to .choices__item-text
rg "choices__item-text" . --type html --type php --type js
echo "---"
# Check if Choices is imported and what version/configuration
rg "Choices\|choices\.js" "app/Views/libri/partials/book_form.php" -B 5 -A 5 | head -30Repository: fabiodalez-dev/Pinakes
Length of output: 8039
🏁 Script executed:
# Get exact context around line 2229 to confirm if it's still the unfixed version
sed -n '2215,2245p' "app/Views/libri/partials/book_form.php" | cat -nRepository: fabiodalez-dev/Pinakes
Length of output: 2030
Il campo serie committerà il suggerimento sbagliato se l'utente digita un nome diverso da quello evidenziato e preme Enter.
Quando la lista dropdown ha un elemento evidenziato (.is-highlighted), il codice delega a origEnter() che può selezionare il suggerimento sbagliato, anche se l'utente ha digitato un valore diverso. Questo causa il salvataggio di dati errati nel campo hidden.
La correzione è già implementata correttamente in initializePublishersChoices() (estrae il testo evidenziato, lo confronta con quello digitato, previene il default se diverso). Applica lo stesso pattern al metodo _onEnterKey della serie autocomplete.
Correzione proposta
choice._onEnterKey = function (event, hasActiveDropdown) {
const typed = input ? input.value.trim() : '';
const dd = wrapper ? wrapper.querySelector('.choices__list--dropdown') : null;
const hl = dd ? dd.querySelector('.choices__item--selectable.is-highlighted') : null;
- if (typed && !hl) { event.preventDefault(); commitTyped(typed); return; }
+ if (typed && hl) {
+ const nameEl = hl.querySelector('.choices__item-text') || hl.childNodes[0];
+ const highlightedText = (nameEl ? nameEl.textContent : hl.textContent).trim().toLowerCase();
+ if (highlightedText !== typed.toLowerCase()) {
+ event.preventDefault();
+ commitTyped(typed);
+ return;
+ }
+ }
+ if (typed && !hl) { event.preventDefault(); commitTyped(typed); return; }
return origEnter(event, hasActiveDropdown);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (typeof choice._onEnterKey === 'function') { | |
| const origEnter = choice._onEnterKey.bind(choice); | |
| choice._onEnterKey = function (event, hasActiveDropdown) { | |
| const typed = input ? input.value.trim() : ''; | |
| const dd = wrapper ? wrapper.querySelector('.choices__list--dropdown') : null; | |
| const hl = dd ? dd.querySelector('.choices__item--selectable.is-highlighted') : null; | |
| if (typed && !hl) { event.preventDefault(); commitTyped(typed); return; } | |
| return origEnter(event, hasActiveDropdown); | |
| }; | |
| if (typeof choice._onEnterKey === 'function') { | |
| const origEnter = choice._onEnterKey.bind(choice); | |
| choice._onEnterKey = function (event, hasActiveDropdown) { | |
| const typed = input ? input.value.trim() : ''; | |
| const dd = wrapper ? wrapper.querySelector('.choices__list--dropdown') : null; | |
| const hl = dd ? dd.querySelector('.choices__item--selectable.is-highlighted') : null; | |
| if (typed && hl) { | |
| const nameEl = hl.querySelector('.choices__item-text') || hl.childNodes[0]; | |
| const highlightedText = (nameEl ? nameEl.textContent : hl.textContent).trim().toLowerCase(); | |
| if (highlightedText !== typed.toLowerCase()) { | |
| event.preventDefault(); | |
| commitTyped(typed); | |
| return; | |
| } | |
| } | |
| if (typed && !hl) { event.preventDefault(); commitTyped(typed); return; } | |
| return origEnter(event, hasActiveDropdown); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/Views/libri/partials/book_form.php` around lines 2229 - 2237, The
_onEnterKey override for the series field does not handle the case where the
user types a value different from the highlighted dropdown item. The current
logic only commits the typed value when there is no highlighted item, but
delegates to origEnter when a highlighted item exists, which incorrectly selects
the wrong suggestion. Extract the text content of the highlighted item (hl),
compare it with the typed value, and if they differ, call event.preventDefault()
and commitTyped(typed) to prevent the wrong suggestion from being selected. This
should mirror the same pattern already correctly implemented in
initializePublishersChoices() for publisher selection, ensuring the user's typed
input is preserved instead of the highlighted dropdown suggestion when they
differ.
Comment on lines
+690
to
+699
| // Series (collana) — now a Choices.js autocomplete (#179) whose submitted | ||
| // value lives in a hidden input set via the exposed setter. | ||
| await page.evaluate((v) => { | ||
| if (window.__seriesAutocomplete && window.__seriesAutocomplete.collana) { | ||
| window.__seriesAutocomplete.collana(v); | ||
| } else { | ||
| const h = document.getElementById('collana'); | ||
| if (h) h.value = v; | ||
| } | ||
| }, `TestSeries_${RUN_ID}`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Impostazione collana via evaluate bypassa il controllo reale dell’autocomplete
In Lines 692-699 il test non passa dal dropdown/suggestion flow di Choices.js, quindi può non rilevare regressioni UI sull’autocomplete della collana. Conviene usare interazione utente (fill, waitForTimeout, click suggestion) anche qui.
🔧 Fix proposto
- await page.evaluate((v) => {
- if (window.__seriesAutocomplete && window.__seriesAutocomplete.collana) {
- window.__seriesAutocomplete.collana(v);
- } else {
- const h = document.getElementById('collana');
- if (h) h.value = v;
- }
- }, `TestSeries_${RUN_ID}`);
+ const seriesValue = `TestSeries_${RUN_ID}`;
+ const collanaWrapper = page.locator('.choices', { has: page.locator('`#collana_select`') });
+ await collanaWrapper.click();
+ const collanaSearch = collanaWrapper.locator('input[type="search"], .choices__input--cloned').first();
+ await collanaSearch.fill(seriesValue);
+ await page.waitForTimeout(350);
+ await page
+ .locator('.choices__list--dropdown .choices__item--selectable', { hasText: seriesValue })
+ .first()
+ .click();As per path instructions, nei test con Choices.js va usato fill + waitForTimeout + click suggestion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/full-test.spec.js` around lines 690 - 699, The collana field
configuration in the evaluate block bypasses the actual Choices.js autocomplete
interaction flow by directly calling the setter or setting the hidden input
value, which means the test does not verify that the autocomplete widget
functions correctly. Replace the page.evaluate approach with actual user
interactions: use fill to input the value into the collana field, add a
waitForTimeout to allow suggestions to appear, and then click on the matching
suggestion from the dropdown to properly simulate user interaction with the
Choices.js autocomplete widget, consistent with how other autocomplete fields
are tested elsewhere in the test suite.
Source: Path instructions
Comment on lines
+15
to
+20
| test.skip(!ADMIN_EMAIL, 'creds not configured (use /tmp/run-e2e.sh)'); | ||
| page = await browser.newPage(); | ||
| await page.goto(`${BASE}/accedi`); | ||
| await page.fill('input[name="email"]', ADMIN_EMAIL); | ||
| await page.fill('input[name="password"]', ADMIN_PASS); | ||
| await page.click('button[type="submit"]'); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guardia credenziali incompleta nel bootstrap della suite
A Line 15 lo skip controlla solo E2E_ADMIN_EMAIL: se E2E_ADMIN_PASS è vuota, i test non vengono skippati e falliscono nel login. Conviene estendere la condizione di skip almeno a email+password admin.
🔧 Fix proposto
test.beforeAll(async ({ browser }) => {
- test.skip(!ADMIN_EMAIL, 'creds not configured (use /tmp/run-e2e.sh)');
+ test.skip(
+ !ADMIN_EMAIL || !ADMIN_PASS,
+ 'E2E admin credentials not configured (set E2E_ADMIN_EMAIL and E2E_ADMIN_PASS)',
+ );
page = await browser.newPage();Based on learnings, in questo repository il gating E2E deve basarsi sulle variabili d’ambiente CI realmente iniettate, non su condizioni parziali.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| test.skip(!ADMIN_EMAIL, 'creds not configured (use /tmp/run-e2e.sh)'); | |
| page = await browser.newPage(); | |
| await page.goto(`${BASE}/accedi`); | |
| await page.fill('input[name="email"]', ADMIN_EMAIL); | |
| await page.fill('input[name="password"]', ADMIN_PASS); | |
| await page.click('button[type="submit"]'); | |
| test.skip( | |
| !ADMIN_EMAIL || !ADMIN_PASS, | |
| 'E2E admin credentials not configured (set E2E_ADMIN_EMAIL and E2E_ADMIN_PASS)', | |
| ); | |
| page = await browser.newPage(); | |
| await page.goto(`${BASE}/accedi`); | |
| await page.fill('input[name="email"]', ADMIN_EMAIL); | |
| await page.fill('input[name="password"]', ADMIN_PASS); | |
| await page.click('button[type="submit"]'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/issue-179-series-autocomplete.spec.js` around lines 15 - 20, The
test.skip condition at the beginning of the suite only validates ADMIN_EMAIL but
does not validate ADMIN_PASS, which means the test will run and fail during the
login attempt if the password credential is missing. Extend the skip condition
to check that both ADMIN_EMAIL and ADMIN_PASS environment variables are properly
configured before allowing the test to execute, ensuring complete credential
validation before attempting the page.fill calls for email and password fields.
Source: Learnings
Comment on lines
+154
to
+166
| // The universe/group/cycle/series fields are Choices.js autocompletes (#179); | ||
| // their submitted value lives in a hidden input set via the exposed setter. | ||
| async function setSeriesAutocomplete(page, field, value) { | ||
| await page.evaluate(([f, v]) => { | ||
| if (window.__seriesAutocomplete && window.__seriesAutocomplete[f]) { | ||
| window.__seriesAutocomplete[f](v); | ||
| } else { | ||
| const h = document.getElementById(f); | ||
| if (h) h.value = v; | ||
| } | ||
| }, [field, value || '']); | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
setSeriesAutocomplete bypassa il percorso E2E reale di Choices.js
In Lines 157-164 il valore viene iniettato via page.evaluate/hidden input: così i test possono passare anche se fetch suggerimenti, dropdown o selezione sono rotti. Qui va simulato il flusso utente (apertura wrapper, fill, attesa breve, click su suggestion) e poi riusato nelle chiamate di Lines 171-176, 351, 395.
🔧 Fix proposto
async function setSeriesAutocomplete(page, field, value) {
- await page.evaluate(([f, v]) => {
- if (window.__seriesAutocomplete && window.__seriesAutocomplete[f]) {
- window.__seriesAutocomplete[f](v);
- } else {
- const h = document.getElementById(f);
- if (h) h.value = v;
- }
- }, [field, value || '']);
+ const v = String(value || '').trim();
+ if (!v) {
+ await page.evaluate((f) => {
+ const h = document.getElementById(f);
+ if (h) h.value = '';
+ }, field);
+ return;
+ }
+
+ const sel = `#${field}_select`;
+ const wrapper = page.locator('.choices', { has: page.locator(sel) });
+ await wrapper.click();
+ const search = wrapper.locator('input[type="search"], .choices__input--cloned').first();
+ await search.fill(v);
+ await page.waitForTimeout(350);
+ await page
+ .locator('.choices__list--dropdown .choices__item--selectable', { hasText: v })
+ .first()
+ .click();
}As per path instructions, nei test con Choices.js va usato fill + waitForTimeout + click suggestion.
Also applies to: 167-176, 351-351, 395-395
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/series-cycles.spec.js` around lines 154 - 166, The
setSeriesAutocomplete function bypasses the actual Choices.js end-to-end flow by
directly injecting values via page.evaluate and hidden input manipulation, which
means tests can pass even if Choices.js functionality like fetching suggestions,
dropdown opening, or selection is broken. Replace the current implementation
with a simulation of the real user interaction flow: fill the input field with
the value, wait briefly for suggestions to appear, and then click on the
matching suggestion. This updated approach should then be reused consistently
wherever setSeriesAutocomplete is called throughout the test file (in the test
cases around lines 171-176, 351, and 395).
Source: Path instructions
Owner
Author
|
Espanso il PHPDoc di |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #179. The cycle/universe/series book-form fields were free-text — the user retyped the universe every time and a typo silently spawned a duplicate universe. They now get the same Choices.js autocomplete as the author/publisher pickers.
Backend —
CollaneController::searchApigains a whitelistedfieldparam mapping each field to its real source:serie_padre→existing universes (collane.nome WHERE tipo='universo'),gruppo_serie→collane.gruppo_serie,ciclo_serie→collane.ciclo,collana→collane.nome+libri.collana. Tables/columns whitelisted, values bound.Frontend — each field is a single-select Choices mirrored into a hidden input (submits unchanged). Typing ≥2 letters proposes existing values; the typed text is always offered + committed on blur/Enter so a new value can still be created. The ISBN scraper fills collana via the exposed setter. New i18n key ×4 locales.
Tests — new
issue-179regression (suggest existing + create new, all 4 fields);series-cycles+full-testmigrated. Validated: PHPStan L5, issue-179 5/5, series-cycles 15/15.Summary by CodeRabbit
New Features
Documentation
Tests