Skip to content

Fix Table Aggregation Separator being set to undefined if it matches default "; "#5240

Merged
alesan99 merged 3 commits into
productionfrom
issue-5154
Aug 28, 2024
Merged

Fix Table Aggregation Separator being set to undefined if it matches default "; "#5240
alesan99 merged 3 commits into
productionfrom
issue-5154

Conversation

@alesan99

@alesan99 alesan99 commented Aug 21, 2024

Copy link
Copy Markdown
Contributor

Fixes #5154

Table Aggregation Separators should no longer be getting set to undefined during serialization if they match the default ("; ").

All previously affected databases will need to have their separators restored separately. Though it is safe to assume all blanks were originally "; ".

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

Testing instructions

  • Go to App Resources -> Record Formatters -> and open any Table Aggregation
  • Set the separator to "; "
  • Save
  • Verify the separator is still there and isn't blank.

@alesan99
alesan99 marked this pull request as ready for review August 26, 2024 14:14
@alesan99
alesan99 requested review from a team August 26, 2024 14:14

@pashiav pashiav left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • Verify the separator is still there and isn't blank.

Working as expected. Good job!

edge

Screen.Recording.2024-08-26.at.1.33.59.PM.mov

pr

Screen.Recording.2024-08-26.at.1.58.08.PM.mov

@pashiav
pashiav requested a review from a team August 26, 2024 19:30

@Areyes42 Areyes42 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Testing instructions

  • Go to App Resources -> Record Formatters -> and open any Table Aggregation
  • Set the separator to "; "
  • Save
  • Verify the separator is still there and isn't blank.

Separator gets retained now, nice job!
Screenshot 2024-08-26 at 4 21 14 PM

@Areyes42
Areyes42 requested a review from a team August 26, 2024 21:23

@maxpatiiuk maxpatiiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@grantfitzsimmons
grantfitzsimmons requested a review from a team August 26, 2024 23:49

@emenslin emenslin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Testing instructions

  • Go to App Resources -> Record Formatters -> and open any Table Aggregation
  • Set the separator to "; "
  • Save
  • Verify the separator is still there and isn't blank.

Looks good, separator doesn't get deleted

@alesan99
alesan99 merged commit f58f0d4 into production Aug 28, 2024
@alesan99
alesan99 deleted the issue-5154 branch August 28, 2024 12:59
@specifysoftware

Copy link
Copy Markdown

This pull request has been mentioned on Specify Community Forum. There might be relevant details there:

https://discourse.specifysoftware.org/t/specify-7-9-7-release-announcement/1979/1

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.

ALL Table Aggregation separators with a space at the end or beginning are ERASED after any changes to DataObjFormatters

7 participants