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

Add series tool in query builder #4952

Open
wants to merge 36 commits into
base: production
Choose a base branch
from
Open

Add series tool in query builder #4952

wants to merge 36 commits into from

Conversation

CarolineDenis
Copy link
Contributor

@CarolineDenis CarolineDenis commented May 24, 2024

Fixes #2000
Fixes #6166

Checklist

  • Self-review the PR after opening it to make sure the changes look good
    and self-explanatory (or properly documented)
  • Add relevant issue to release milestone

Testing instructions

  • Open the QB with CO table
  • Add catalog num field and some other field
  • verify series checkbox is present only when Cat num is one of the field
  • Check Series
  • verify Distinct is read only
  • Uncheck Series
  • Verify you can check Distinct and that Series become readonly
  • Uncheck Distinct and check Series
  • Run the query
  • Verify that all the records that have the same data for each field in your query and that have consecutive cat num are group together
  • Verify that the consecutive cat numbers are displayed as a range

Tips:

  • Create a new CO with a determination and a preparation in a db with auto-numbering for cat num
  • Save
  • Use bulk Carry and create 5 identical copies
  • Create another new CO with all the exact same info as the previous one but skip one cat num
  • This will give you some record to test with series

With Series
Screenshot 2025-02-03 at 8 50 57 AM

Without Series
Screenshot 2025-02-03 at 8 50 49 AM

@CarolineDenis CarolineDenis changed the title Issue 2000 Add series tool in query builder May 24, 2024
@CarolineDenis CarolineDenis added this to the 7.9.x milestone Jun 18, 2024
@grantfitzsimmons
Copy link
Member

This is the second component of #4804

@realVinayak
Copy link
Contributor

FYI: backend, after #4929, will support server-side record cloning

@CarolineDenis CarolineDenis marked this pull request as ready for review February 3, 2025 16:53
@CarolineDenis CarolineDenis requested review from sharadsw and a team February 3, 2025 16:55
@CarolineDenis CarolineDenis modified the milestones: 7.x, 7.10.1, 7.10.2 Feb 24, 2025
@grantfitzsimmons
Copy link
Member

@specify/ux-testing It would be great if you could use the same database with Specify 6 to make sure the results are consistent between versions. If they are not, we should try and understand what the difference is.

Copy link
Contributor

@pashiav pashiav left a comment

Choose a reason for hiding this comment

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

  • verify series checkbox is present only when Cat num is one of the field
  • verify Distinct is read only
  • Verify you can check Distinct and that Series become readonly
  • Verify that all the records that have the same data for each field in your query and that have consecutive cat num are group together
  • Verify that the consecutive cat numbers are displayed as a range

Issues

1. Any operator on catalogNumber does not work.

Screen.Recording.2025-02-26.at.9.45.20.AM.mov

  1. Ordering does not work with Series checked.
    UPDATE: Will not be included in this PR. Written up issue: Ordering does not work with Series checked #6291
Screen.Recording.2025-02-26.at.8.50.28.AM.mov

3. "Error: Returned 328 results, when expected at most 40"
Specify 7 Crash Report - 2025-02-26T15_07_29.529Z.txt
Screenshot 2025-02-26 at 9 07 28 AM

"Error: Returned 143 results, when expected at most 40":
Specify 7 Crash Report - 2025-02-26T15_20_36.183Z.txt
I reproduced this error with this query:
Screenshot 2025-02-26 at 9 20 45 AM


4. - [ ] Verify that all the records that have the same data for each field in your query and that have consecutive cat num are group together

  • Can't really test this on fields from the Tip because of the error from issue 3.
  • Also noticed that the results show up as "Results: (1)" for the query catalogNumber -> Any - is this expected?
Screenshot 2025-02-26 at 9 53 26 AM

Also I tested locally because on the test panel I get this error upon opening the instance:
Specify 7 Crash Report - 2025-02-26T15_13_30.978Z.txt

Screenshot 2025-02-26 at 9 13 37 AM

@pashiav pashiav requested a review from a team February 26, 2025 15:56
@acwhite211
Copy link
Member

acwhite211 commented Feb 27, 2025

Found an error when there are letters in the Catalog Number field. Right now, the series grouping logic evaluates adjacent Catalog Numbers by converting them to integers, so there is an error when there are letters. Working on a fix.
image
image

@CarolineDenis
Copy link
Contributor Author

@pashiav for your comment:
2. Ordering does not work with Series checked.

It will not be addressed in this pr. Could you create a new issue and link it in your comment please?

@acwhite211
Copy link
Member

acwhite211 commented Feb 27, 2025

I fixed the series logic for alphanumeric cat nums. I was able to get the sorting of the series ranged cat nums working as well 😀

Copy link
Collaborator

@emenslin emenslin left a comment

Choose a reason for hiding this comment

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

  • verify series checkbox is present only when Cat num is one of the field
  • verify Distinct is read only
  • Verify you can check Distinct and that Series become readonly
  • Verify that all the records that have the same data for each field in your query and that have consecutive cat num are group together
  • Verify that the consecutive cat numbers are displayed as a range

Adding determinations, modified by agent, or age to the query throws an error

02-28_14.24.mp4

Specify 7 Crash Report - 2025-02-28T20_47_01.573Z.txt

Results showing incorrect count

02-28_14.32.mp4

If you check series and then delete the Cat # from the query and try to query you get an error

02-28_14.37.mp4

Specify 7 Crash Report - 2025-02-28T20_45_22.214Z.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Dev Attention Needed
Development

Successfully merging this pull request may close these issues.

Add series tool in the QB Query builder does not support "Always","Prompt" or "Series results" checkboxes
7 participants