Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(insert_many): list instead of set to maintain order #18641

Merged
merged 3 commits into from Oct 28, 2022

Conversation

netchampfaris
Copy link
Contributor

@netchampfaris netchampfaris commented Oct 28, 2022

insert_many takes a list of docs and it used to return the names of these docs in order. While this PR improved child doc handling, it broke the order in which doc names were returned because it uses set() and it doesn't guarantee order.

This PR fixes the order of doc names.

Before:
image

After:
image

@netchampfaris netchampfaris requested a review from a team as a code owner October 28, 2022 08:20
@netchampfaris netchampfaris requested review from phot0n and removed request for a team October 28, 2022 08:20
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Oct 28, 2022
@netchampfaris netchampfaris removed the add-test-cases Add test case to validate fix or enhancement label Oct 28, 2022
ankush
ankush previously approved these changes Oct 28, 2022
@phot0n
Copy link
Contributor

phot0n commented Oct 28, 2022

can we also add an assertion to make sure the api return order is correct in insert_many api?

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

Merging #18641 (e5937bd) into develop (c44e278) will decrease coverage by 0.27%.
The diff coverage is 81.48%.

❗ Current head e5937bd differs from pull request most recent head d285df9. Consider uploading reports for the commit d285df9 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #18641      +/-   ##
===========================================
- Coverage    63.00%   62.73%   -0.28%     
===========================================
  Files          746      747       +1     
  Lines        67391    68432    +1041     
  Branches      5992     5975      -17     
===========================================
+ Hits         42463    42929     +466     
- Misses       21438    22033     +595     
+ Partials      3490     3470      -20     
Flag Coverage Δ
server-mariadb 67.16% <94.73%> (-0.03%) ⬇️
server-postgres 67.17% <94.73%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@phot0n phot0n merged commit aa8957e into frappe:develop Oct 28, 2022
mergify bot pushed a commit that referenced this pull request Oct 28, 2022
phot0n added a commit that referenced this pull request Oct 28, 2022
…-18641

fix(insert_many): list instead of set to maintain order (backport #18641)
frappe-pr-bot pushed a commit that referenced this pull request Nov 1, 2022
# [14.14.0](v14.13.0...v14.14.0) (2022-11-01)

### Bug Fixes

* correct stacklevel for warnings ([#18633](#18633)) ([#18634](#18634)) ([17821f1](17821f1))
* **formatting:** use snake case for variable names ([32851ce](32851ce))
* handle exceptions thrown in post_schema_updates ([#18648](#18648)) ([#18663](#18663)) ([29a659a](29a659a))
* **insert_many:** list instead of set to maintain order ([#18641](#18641)) ([4c521b8](4c521b8))
* make module def custom checkbox readonly in prod mode ([#18698](#18698)) ([18fa732](18fa732))
* **router-js:** handle case when link is not of same host ([1b70ec4](1b70ec4))
* set proper cache key for singles when name is passed as `None` ([#18667](#18667)) ([#18668](#18668)) ([e32ba96](e32ba96))
* Signature form ([#18477](#18477)) ([8e4711b](8e4711b))
* Signature form (backport [#18477](#18477)) ([#18690](#18690)) ([34c1611](34c1611))
* support symlinked /files directory ([#18702](#18702)) ([7b8cbd0](7b8cbd0))
* use `async...await` when parsing route ([9c09c65](9c09c65))
* use `is_file_path_valid` instead of `is_safe_path` ([#18316](#18316)) ([#18642](#18642)) ([537966c](537966c))
* **UX:** Info message on workspace for better UX (backport [#18701](#18701)) ([#18703](#18703)) ([5ba85ec](5ba85ec))
* **UX:** session expiry explanation and defaults ([#18680](#18680)) ([#18685](#18685)) ([515f6c2](515f6c2))
* **UX:** warn about Etc/* timezones behaviour ([#18587](#18587)) ([#18588](#18588)) ([c8728f5](c8728f5))
* webform validation script not working ([ab2b8df](ab2b8df))

### Features

* add video conferencing option (Google Meet) to Google Calendar integration (backport [#17851](#17851)) ([#18456](#18456)) ([b04a54c](b04a54c))
* support list view for "show titles instead of name" ([#18060](#18060)) ([#18681](#18681)) ([64c2555](64c2555))

### Performance Improvements

* ensure cache works for `non_standard_user_types` when empty ([#18665](#18665)) ([#18669](#18669)) ([5f7928b](5f7928b))

### Reverts

* Revert "fix: use `is_file_path_valid` instead of `is_safe_path` (#18316) (#18642)" (#18696) ([a08c029](a08c029)), closes [#18316](#18316) [#18642](#18642) [#18696](#18696)
stephenBDT pushed a commit to alias/frappe that referenced this pull request Nov 7, 2022
stephenBDT pushed a commit to alias/frappe that referenced this pull request Nov 7, 2022
# [14.14.0](frappe/frappe@v14.13.0...v14.14.0) (2022-11-01)

### Bug Fixes

* correct stacklevel for warnings ([frappe#18633](frappe#18633)) ([frappe#18634](frappe#18634)) ([17821f1](frappe@17821f1))
* **formatting:** use snake case for variable names ([32851ce](frappe@32851ce))
* handle exceptions thrown in post_schema_updates ([frappe#18648](frappe#18648)) ([frappe#18663](frappe#18663)) ([29a659a](frappe@29a659a))
* **insert_many:** list instead of set to maintain order ([frappe#18641](frappe#18641)) ([4c521b8](frappe@4c521b8))
* make module def custom checkbox readonly in prod mode ([frappe#18698](frappe#18698)) ([18fa732](frappe@18fa732))
* **router-js:** handle case when link is not of same host ([1b70ec4](frappe@1b70ec4))
* set proper cache key for singles when name is passed as `None` ([frappe#18667](frappe#18667)) ([frappe#18668](frappe#18668)) ([e32ba96](frappe@e32ba96))
* Signature form ([frappe#18477](frappe#18477)) ([8e4711b](frappe@8e4711b))
* Signature form (backport [frappe#18477](frappe#18477)) ([frappe#18690](frappe#18690)) ([34c1611](frappe@34c1611))
* support symlinked /files directory ([frappe#18702](frappe#18702)) ([7b8cbd0](frappe@7b8cbd0))
* use `async...await` when parsing route ([9c09c65](frappe@9c09c65))
* use `is_file_path_valid` instead of `is_safe_path` ([frappe#18316](frappe#18316)) ([frappe#18642](frappe#18642)) ([537966c](frappe@537966c))
* **UX:** Info message on workspace for better UX (backport [frappe#18701](frappe#18701)) ([frappe#18703](frappe#18703)) ([5ba85ec](frappe@5ba85ec))
* **UX:** session expiry explanation and defaults ([frappe#18680](frappe#18680)) ([frappe#18685](frappe#18685)) ([515f6c2](frappe@515f6c2))
* **UX:** warn about Etc/* timezones behaviour ([frappe#18587](frappe#18587)) ([frappe#18588](frappe#18588)) ([c8728f5](frappe@c8728f5))
* webform validation script not working ([ab2b8df](frappe@ab2b8df))

### Features

* add video conferencing option (Google Meet) to Google Calendar integration (backport [frappe#17851](frappe#17851)) ([frappe#18456](frappe#18456)) ([b04a54c](frappe@b04a54c))
* support list view for "show titles instead of name" ([frappe#18060](frappe#18060)) ([frappe#18681](frappe#18681)) ([64c2555](frappe@64c2555))

### Performance Improvements

* ensure cache works for `non_standard_user_types` when empty ([frappe#18665](frappe#18665)) ([frappe#18669](frappe#18669)) ([5f7928b](frappe@5f7928b))

### Reverts

* Revert "fix: use `is_file_path_valid` instead of `is_safe_path` (frappe#18316) (frappe#18642)" (frappe#18696) ([a08c029](frappe@a08c029)), closes [frappe#18316](frappe#18316) [frappe#18642](frappe#18642) [frappe#18696](frappe#18696)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants