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

Batch Edit: Use scoped picklist for formatting in batch edit #6344

Merged
merged 1 commit into from
Mar 20, 2025

Conversation

sharadsw
Copy link
Contributor

@sharadsw sharadsw commented Mar 19, 2025

Fixes #6339

Notes in #6339 (comment)

Checklist

  • Self-review the PR after opening it to make sure the changes look good and
    self-explanatory (or properly documented)
  • Add automated tests
  • Add relevant issue to release milestone
  • Add relevant documentation (Tester - Dev)
  • Add a reverse migration if a migration is present in the PR

Testing instructions

  • On ojsmnh use collection Fossil Invertebrates
  • Make an Accession Query with fields Type and Status
  • Click Batch Edit
  • Verify picklist titles are being used
  • General test picklists in Batch Edit with other tables
  • General test picklists in Workbench

@sharadsw sharadsw added this to the 7.10.2 milestone Mar 20, 2025
@sharadsw sharadsw marked this pull request as ready for review March 20, 2025 14:20
@sharadsw sharadsw requested review from a team March 20, 2025 14:20
Copy link

@bronwyncombs bronwyncombs left a comment

Choose a reason for hiding this comment

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

  • Verify picklist titles are being used
Screenshot 2025-03-20 at 11 37 41 AM Screenshot 2025-03-20 at 11 38 03 AM Screenshot 2025-03-20 at 11 39 53 AM
  • General test picklists in Batch Edit with other tables
Screenshot 2025-03-20 at 11 49 10 AM
  • General test picklists in Workbench
Screenshot 2025-03-20 at 11 54 18 AM

I also want to comment on an existing behavior that came up during testing but already exists. If if you save a mapping, proceed to the data set interface, then later go back to data mapper and change fields and/or the base table, the column titles reflect the initial fields mapped to the columns in the data mapper and data set. However, validation still catches "illegal" pick list values and reflects the updated options so this seems like a front end only discrepancy.

@sharadsw sharadsw merged commit 0bb287a into production Mar 20, 2025
12 checks passed
@sharadsw sharadsw deleted the issue-6339 branch March 20, 2025 17:37
@melton-jason
Copy link
Contributor

Nice work! 😎
No new problems here, just further documenting/explaining the caveats and drawbacks of the current solution, some potential fixes, and how the current solution might still be the best without fundamentally changing how PickLists work and are scoped.

  • Accession is scoped at a higher level than collection - which means Queries will display the same Accession records regardless of which collection you are in.
    • If we scope picklist formatting by collection, there will be some collections where the formatter doesn't look at the 'correct' picklist for Accession because each collection has a different one
  • In other tables, scoping picklist formatter by collection will not cover cases where a user somehow has an invalid entry in a picklist field for that collection

From #6339 (comment) by @sharadsw

Yeah, not sure the best way to handle this would be? If we want to keep PickLists scoped at the Collection level, our options are pretty limited. If this is something we want to address, I imagine we could maybe try:

  • (For tables scoped above Collection?) Keeping the initial PickList query filtering as unscoped as possible on the BackEnd, and try to aggregate/merge the PickLists. In other words, titles/values not present in one PickList could be "merged" into the other and the aggregated PickList items be used for validation purposes
    • Although the "merge" process itself could be very complicated, especially when considering case sensitivity (i.e., is collection=Collection in most cases?) , possible separators between words (space vs. underscore vs. hyphen), etc.
    • Essentially, this would be expanding a "valid" value for a field with a PickList to be all values for PickLists across Collections (within the same Discipline? Or among all PickLists with the same name for PickLists assigned to the field?)

To demonstrate/document the current behavior (in the QB as well, which was fixes as well with this change), I have two Collections in the same Discipline: Fish and Fish2, and will be considering an AccessionType picklist set on the field Accession -> type, which is defined with different titles + values in each Collection (both are largely based from the default AccessionType picklists with small modifications)

The AccessionType PickList in the Fish Collection:

Title Value
Field Work field_work
Collection cln

AccessionType in the Fish2 Collection:

Title Value
Field Work field_work
Collection collection
Finding Nemo I_shall_call_him_squishy

A Query from the Fish Collection

fish_collection_query

A Query from the Fish2 Collection

fish2_collection_query

Initiating a Batch Edit from the Fish Collection

fish_collection_batch_edit.mov

This example is relatively tame - and probably the most common- compared to what is possible (and technically valid) in Specify. Imagine a case where one or more PickLists across Collections have different types! For Example, one of the Collection's AccessionType PickList could instead be of type Value From Field, or even of type Entire Table with a formatter.

The current solution is simple (albeit while being very restrictive in some cases), so definitely worth talking about changing (the design + implementation of PickLists from Specify 6) in the future!

@melton-jason
Copy link
Contributor

melton-jason commented Mar 20, 2025

If if you save a mapping, proceed to the data set interface, then later go back to data mapper and change fields and/or the base table, the column titles reflect the initial fields mapped to the columns in the data mapper and data set

From #6344 (review) by @bronwyncombs

Yeah, I'm pretty sure the intention of the feature is actually to be helpful and not override any user-specified column names, but it oftentimes falls short in some cases --- especially in a testing environment when the data within Data Sets (and particularly the "type" of data within each column) is variable.

Imagine you are given or have created a spreadsheet representing some Specify-external data of specimens. You would theoretically be very familiar and comfortable with exactly what each column is is supposed to represent by name alone.
When you import the spreadsheet using the WorkBench, you have to figure out (or are somehow otherwise given) how the columns from the spreadsheet map to fields in Specify.
Even after mapping the fields, if you were only given the Specify mapping of a column and not the spreadsheet name for that column, you may need to look at the data, consult some external/given source, and/or perform slightly more mental gymnastics to make the connection that: Specify mapping = specific column in the spreadsheet.

The core assumption is that the user-provided spreadsheet column names would be more comfortable and familiar than the autogenerated Specify mapping.

When you explicitly add a new column to a Data Set and map it to a field in Specify, Specify always knows that specific column is brand-new and so it autogenerates a name for the column based on the mapping.
When you edit the mapping of an existing column, Specify doesn't know if that column (name) came from something autogenerated, from a spreadsheet, or some other user-provided external source so it errs on the safe side and doesn't overwrite it: assuming the "type" of data within that column hasn't changed, and the user is more comfortable with the existing column name to describe the data.

You're right in that it's a visual-only discrepancy/effect (the column names), and doesn't actually affect how the data is validated and processed.

@specify/ux-testing @bronwyncombs
What would you propose as a solution?

From the problem, I can think of two potential solutions, both of which could potentially be implemented:

  • Allow the columns to be renamed in the "Data Set" view
    • Probably ideally via an option from right-clicking the column in the Data Set View (i.e., added to the column context menu)?
    • In a side note, from an accessibility standpoint, is the column context menu (easily and intuitively) accessible using only mouse and keyboard?
  • Keep track of where each column came from. If a column name was autogenerated from the mapper previously, rename it whenever the column is re-mapped in the Field Mapping view.

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

Successfully merging this pull request may close these issues.

Batch edit picklist using values
5 participants