Skip to content

Fixes issue with concatenated nrps url param#69918

Draft
nicklathe wants to merge 135 commits intostagingfrom
nlathe/nrps-query-fix
Draft

Fixes issue with concatenated nrps url param#69918
nicklathe wants to merge 135 commits intostagingfrom
nlathe/nrps-query-fix

Conversation

@nicklathe
Copy link
Copy Markdown
Contributor

  • De-duplicates possible dupplicated params as part of the NRPS response from certain LMS platforms

Warning!!

We have entered Pixel Lock for Hour of AI! All merges to the staging branch from Dec 2 through Dec 12 must go through live change review and be deemed critical for supporting the Hour of AI. External contributions will not be accepted at this time.

For non-critical changes, please change your base to staging-next and delete this warning. We will merge staging-next into staging on Dec 15, 2025.

Links

  • Jira:

Testing story

Deployment strategy

Follow-up work

Privacy

Security

Caching

PR Creation Checklist:

  • Tests provide adequate coverage
  • Privacy impacts have been documented
  • Security impacts have been documented
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Follow-up work items (including potential tech debt) are tracked and linked

sureshc and others added 25 commits December 3, 2025 09:20
[AITT-1297] Ensure personalization intersticials and final gif auto progress uses
* renaming and update accept-reject schema to include folderId and id for each file

* ask for id and folderId of updated file and validate name

* move updating openFiles to helper function, add numeric suffix for file name if duplicate

* add unit tests for getMergedAiTutorCodeWithSource helper function
…ages (#69892)

* reset load error correctly

* Update apps/src/lab2/views/components/Instructions/ResourcePanel/VersionHistory/VersionHistoryPanel.tsx

Co-authored-by: Kelby Hawn <9256643+kelbyhawn@users.noreply.github.com>

---------

Co-authored-by: Kelby Hawn <9256643+kelbyhawn@users.noreply.github.com>
* experimentally set sc tunnel idle timeout to 1s in ci

* explicitly pass http-idle-conn-timeout flag to sc run

* debug SAUCE_HTTP_IDLE_CONN_TIMEOUT

* increase sauce log levels

* also try warn log level

* fix sc log syntax

* verbosely log proxy header output

* dump sauce connect log

* limit http logs to proxy errors

* reduce log level to INFO

* set up bash trap to dump sc log

* introduce an error in angle_helper.feature to make the build fail

* Tail the Sauce Connect log for real-time visibility in CI

* Revert "introduce an error in angle_helper.feature to make the build fail"

This reverts commit f6fff21.

* remove proxy error logging

* Revert "Tail the Sauce Connect log" and go back to trap

This reverts commit 45b94eb.

* remove ineffective idle timeout setting

* Reapply "Tail the Sauce Connect log"

This reverts commit 5ad1c8f.

* use system to invoke tail

* Revert "use system to invoke tail"

This reverts commit 856f25d.

* Revert "Reapply "Tail the Sauce Connect log""

This reverts commit e7d6340.

* rerun

* introduce artificial 5xx error into ui tests

* -m [skip local webdriver]

* Revert "introduce artificial 5xx error into ui tests"

This reverts commit 893deaf.

* increase ulimit per saucelabs warning

* Update schema cache dump after schema changes.

* extract LOG_FILE_PATH

* dump log file using ruby instead

* remove unused daemonize param

* remove one more mention of daemonize

* add dump_logs option

* revert schema cache

* add verbose flag

* rerun drone

---------

Co-authored-by: Continuous Integration <dev@code.org>
…-components

tidy 🧹:  Remove radium from applab components
* Update aidiff message model with potential artifact

* Update version

* Update the version number for message migration
Save podcast script to AiLessonSummary table
* Move unit selector to shared folder

* Fix

* Move the lesson sel and add unit sel

* Add lesson selector

* Fix test

* Add auth

* Fix test

* Add labels
…9893)

* Replace CanEnableAITutor with an experiment and add the AI Settings tab behind the experiment.

* remove unnecessary spread

* Fix teacher nav bar tests
* Fix "with exception" calls to Honeybadger.notify_cronjob_error

* Also update daily_weather to use this call
…ts + stories (#69801)

* feat(component-library): checkbox styles parity with radio (gaps, order, transitions)

* feat(component-library): checkbox add textThickness, ariaLabel, aria-checked mixed, className/children, memo

* feat(component-library): checkbox stories add label weights, multiline, custom content; remove legacy Supernova stories

* chore(prettier): format checkbox.module.scss

* chore(prettier): format Checkbox.tsx

* chore(prettier): format Checkbox.story.tsx

* chore(typecheck): add required checked/onChange to CheckboxesWithCustomContent story

* chore(lint): remove any casts in Checkbox; use typed HTML attributes

* chore(typecheck): avoid reading className from spread HTML attributes in Checkbox

* chore(a11y): align Checkbox aria-label fallback with RadioButton (use ||)

* chore(stories): remove redundant comment from Checkbox stories

* chore(Checkbox): remove memoization from Checkbox component

---------

Co-authored-by: denyslevada <levada.denys@gmail.com>
* fix dropdown persistence issue

* fix comment typos

* clean up

* blur the button focus style

* more clean up

* update comments

* update comments again

* update solution to use custom event

* handle memory leaks
…p UI (#69902)

* add AiTutorVersionFileChip - wip

* more style updates

* use senary for dark mode improvement

* add file chips to action notification

* add accepted/rejected icon

* add functionality to handlePreviewClick

* update description with example of what should be returned for a modified file

* add time stamp to aiFilePathToPreview so that effect is triggered even after user enters url manually

* add JS

* update comment, remove logs

* styling updates
  - De-duplicates possible dupplicated params as part of the NRPS
    response from certain LMS platforms

Signed-off-by: Nick Lathe <nick.lathe@code.org>
assert_equal 150, res[:members].length
end

test 'handles duplicate query parameters correctly' do
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is failing, still poking at it.

raise ArgumentError unless client_id && issuer
@client_id = client_id
@issuer = issuer
@page_limit = PAGE_LIMIT
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we weren't overriding this via the method parameter, I thought it might be safer to declare it as a const here. It's also cleaner than passing it down to the build_uri method

@page_2_url = 'https://example.com/api/lti/courses/1234/memberships?rlid=1234&page=2'
@page_3_url = 'https://example.com/api/lti/courses/1234/memberships?rlid=1234&page=3'
@page_2_url = 'https://example.com/api/lti/courses/1234/memberships?limit=100&page=2&rlid=1234'
@page_3_url = 'https://example.com/api/lti/courses/1234/memberships?limit=100&page=3&rlid=1234'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of these query params changed now that we're calling .merge. My guess is that method orders the keys alphabetically, but I haven't looked into that yet.

@nicklathe nicklathe changed the base branch from staging-next to staging December 5, 2025 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.