Fixup: unselection of items from AJAX data sources with non-string identifiers by jayaddison · Pull Request #6241 · select2/select2

jayaddison · 2023-04-05T15:23:47Z

This pull request includes a

Bug fix

The following changes were made

Adds the jquery-mockjax plugin as a vendored integration test dependency.
Provides integration test coverage for selection-then-unselection of items based on an AJAX data source.
Performs normalization of result items received from AJAX sources.

Resolves #6128.

…select controls

…log events so that info-level messages do not pollute the test log output

kevin-brown · 2023-04-06T23:19:30Z

Maybe-controversial: removes some special-case handling for multi-select controls

Yeah those are too controversial to roll in here. We intentionally don't have single-select elements issue unselect because that'd always be paired with a select.

kevin-brown · 2023-04-06T23:26:57Z

I am likely going to spin a new ajax-tests.js module out of this and move this test into it, since we don't currently have good tests around AJAX handling and this would be a solid first test to add. More than likely I would switch this to just be testing the AjaxAdapter itself and create a new test around the weirdness with creating <option>s on select events.

…n multi-select controls" This reverts commit 8fd9a00.

jayaddison · 2023-04-07T09:23:42Z

Thanks @kevin-brown - how would you like to co-ordinate the creation of that test file? (should I attempt to create it here, or would you prefer to do that separately and move these changes into it?)

…ray' check condition

jayaddison · 2023-04-07T09:37:30Z

Yeah those are too controversial to roll in here. We intentionally don't have single-select elements issue unselect because that'd always be paired with a select.

Sounds good; the single-select unselect event change here has been reverted.

jayaddison · 2023-04-07T09:59:04Z

Also a note: the jquery-mockjax component is available under dual MIT-or-GPLv2 licensing terms (it's up to the authors of the codebase where it is included to decide on the applicable terms). I think I did check the license compatibility before choosing it for this branch, but wanted to double-check that again anyway.

kevin-brown · 2023-04-07T18:12:35Z

Good call on checking the license, thank you for switching back the unselect changes. This looks good to go from a high level, so I'll mark it for my next sweep.

I'll take care of the testing changes independently, the tests you wrote get the job done for this change set. Anything I add would likely be in addition to the ones here and aren't required.

jayaddison · 2023-06-23T17:13:17Z

Hey @kevin-brown - I'm afraid I'm joining the chorus of 'any updates on this' here - is there anything I can do to help this and/or the 4.1.0 milestone along?

…esults

jayaddison · 2024-03-28T17:23:01Z

Note, partly since licensing was discussed here previously: jquery-mockjax v2.6.1 has consolidated from dual MIT-and-GPLv2 licensing to a single MIT license (ref: jakerella/jquery-mockjax@8183c94 and the associated changelog entry).

jayaddison · 2024-07-25T21:52:07Z

Hi @kevin-brown - I've been browsing through a few old pull requests to clear the decks - any chance this might be nearish-acceptable? (I've one or two others open also, but this one seemed to be most-progressed). No problem if not.

kevin-brown · 2024-08-23T21:23:47Z

Just realized I merged this in right after we upgraded QUnit so the tests are now failing. Should be a quick fix to update the calls.

* Fixup: reference `QUnit.test` instead of relying on global namespace entry Relates-to / merge-resolution-for #6241, #6334 * Build: recompile distributable files * Tests: temporarily comment-out a failing integration test * Revert "Tests: temporarily comment-out a failing integration test" This reverts commit 1863823. * Tests: isolate `selection:update` integration test event handlers * Cleanup: regenerate NPM lockfile

- Remove '(unreleased)' marker from the 4.1.0 heading - Add new features: jQuery 4.0.0 support (#6332), originalEvent in close trigger args (#6079) - Add bug fixes: placeholder misalignment (#6277), RTL choice remove button (#6257), digit-only data-placeholder (#6297), AJAX unselection with non-string IDs (#6241, #6335), optgroup child string coercion (#6338) - Add translations: lb (#6131), ug (#6166), ar (#6175), zh-TW (#6157), id (#6153), tr (#6123), ro (#6190), de/es/fr/pt/pt-BR (#6132), pl (#6097, #6377), nb (#6213), fa (#6258), nl (#6269), pt-BR typo (#6200), missing bs/ca/da/fi strings (#6305) - Add miscellaneous: native DOM replacements for jQuery attr/removeAttr (#6227, #6228), classList.add (#6229), jQuery removal from Utils and Translation (#6233), prop() removal (#6289), NPM trusted publishing (#6405)

…entifiers (select2#6241) * integration test: selection and clearing of data from ajax source * Fix: normalize response data items received from AJAX requests * Extension: remove special-case logic for unselection events on multi-select controls * test fixup: call assert.async test-completion event handler * test log cleanup: increase the severity threshold for jquery-mockjax log events so that info-level messages do not pollute the test log output * Revert "Extension: remove special-case logic for unselection events on multi-select controls" This reverts commit 8fd9a00. * Relocate AJAX-response-result normalization step, and add 'Array.isArray' check condition * Lint fixup: rearrange code to fit within line length limits * Nitpick: remove trailing end-of-line whitespace * Dependencies: update to mockjax v2.6.1

* Fixup: reference `QUnit.test` instead of relying on global namespace entry Relates-to / merge-resolution-for select2#6241, select2#6334 * Build: recompile distributable files * Tests: temporarily comment-out a failing integration test * Revert "Tests: temporarily comment-out a failing integration test" This reverts commit 1863823. * Tests: isolate `selection:update` integration test event handlers * Cleanup: regenerate NPM lockfile

- Remove '(unreleased)' marker from the 4.1.0 heading - Add new features: jQuery 4.0.0 support (select2#6332), originalEvent in close trigger args (select2#6079) - Add bug fixes: placeholder misalignment (select2#6277), RTL choice remove button (select2#6257), digit-only data-placeholder (select2#6297), AJAX unselection with non-string IDs (select2#6241, select2#6335), optgroup child string coercion (select2#6338) - Add translations: lb (select2#6131), ug (select2#6166), ar (select2#6175), zh-TW (select2#6157), id (select2#6153), tr (select2#6123), ro (select2#6190), de/es/fr/pt/pt-BR (select2#6132), pl (select2#6097, select2#6377), nb (select2#6213), fa (select2#6258), nl (select2#6269), pt-BR typo (select2#6200), missing bs/ca/da/fi strings (select2#6305) - Add miscellaneous: native DOM replacements for jQuery attr/removeAttr (select2#6227, select2#6228), classList.add (select2#6229), jQuery removal from Utils and Translation (select2#6233), prop() removal (select2#6289), NPM trusted publishing (select2#6405)

jayaddison added 5 commits April 5, 2023 16:19

integration test: selection and clearing of data from ajax source

652b5e0

Fix: normalize response data items received from AJAX requests

c013315

Extension: remove special-case logic for unselection events on multi-…

8fd9a00

…select controls

test fixup: call assert.async test-completion event handler

8719f0a

test log cleanup: increase the severity threshold for jquery-mockjax …

9fd6939

…log events so that info-level messages do not pollute the test log output

jayaddison changed the title ~~Test coverage and fixup: unselection of items from AJAX data sources with non-string identifiers~~ Fixup: unselection of items from AJAX data sources with non-string identifiers Apr 6, 2023

kevin-brown requested changes Apr 6, 2023

View reviewed changes

Comment thread src/js/select2/data/ajax.js Outdated

Revert "Extension: remove special-case logic for unselection events o…

d58e370

…n multi-select controls" This reverts commit 8fd9a00.

jayaddison added 2 commits April 7, 2023 10:34

Relocate AJAX-response-result normalization step, and add 'Array.isAr…

96b2f31

…ray' check condition

Lint fixup: rearrange code to fit within line length limits

07d3fab

kevin-brown added enhancement 4.x feature: ajax status: pending PR merge labels Apr 7, 2023

kevin-brown added this to the 4.1.0 milestone Apr 7, 2023

Nitpick: remove trailing end-of-line whitespace

44ae875

jayaddison mentioned this pull request Oct 18, 2023

ajax datasources: allow search result selection to refresh previously-chosen option labels #6282

Closed

3 tasks

jayaddison and others added 2 commits February 22, 2024 15:54

Merge branch 'select2:develop' into issue-6128/unselection-for-ajax-r…

7372800

…esults

Dependencies: update to mockjax v2.6.1

f8596b1

jayaddison requested a review from kevin-brown March 28, 2024 17:23

kevin-brown approved these changes Aug 23, 2024

View reviewed changes

kevin-brown merged commit 0e12916 into select2:develop Aug 23, 2024

jayaddison deleted the issue-6128/unselection-for-ajax-results branch August 23, 2024 21:52

jayaddison mentioned this pull request Aug 23, 2024

Fixups: post-merge updates related to PR #6241 #6335

Merged

3 tasks

madejejej mentioned this pull request Jun 4, 2026

4.1.0: AJAX results.map(_normalizeItem) throws (undefined container) under ESM bundlers #6423

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixup: unselection of items from AJAX data sources with non-string identifiers#6241

Fixup: unselection of items from AJAX data sources with non-string identifiers#6241
kevin-brown merged 11 commits into
select2:developfrom
openculinary:issue-6128/unselection-for-ajax-results

jayaddison commented Apr 5, 2023 •

edited

Loading

Uh oh!

kevin-brown commented Apr 6, 2023

Uh oh!

Uh oh!

kevin-brown commented Apr 6, 2023

Uh oh!

jayaddison commented Apr 7, 2023

Uh oh!

jayaddison commented Apr 7, 2023

Uh oh!

jayaddison commented Apr 7, 2023

Uh oh!

kevin-brown commented Apr 7, 2023

Uh oh!

jayaddison commented Jun 23, 2023

Uh oh!

jayaddison commented Mar 28, 2024

Uh oh!

jayaddison commented Jul 25, 2024

Uh oh!

kevin-brown commented Aug 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jayaddison commented Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kevin-brown commented Apr 6, 2023

Uh oh!

Uh oh!

kevin-brown commented Apr 6, 2023

Uh oh!

jayaddison commented Apr 7, 2023

Uh oh!

jayaddison commented Apr 7, 2023

Uh oh!

jayaddison commented Apr 7, 2023

Uh oh!

kevin-brown commented Apr 7, 2023

Uh oh!

jayaddison commented Jun 23, 2023

Uh oh!

jayaddison commented Mar 28, 2024

Uh oh!

jayaddison commented Jul 25, 2024

Uh oh!

kevin-brown commented Aug 23, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jayaddison commented Apr 5, 2023 •

edited

Loading