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

Ban user column to be used as display column #12003

Merged

Conversation

adrinr
Copy link
Collaborator

@adrinr adrinr commented Oct 9, 2023

Description

Disabling user columns to be set as a display column. They add plenty of complexity, and there are workarounds using formulas instead

Screenshots

image

const subtype = fieldSchema.subtype as FieldSubtype
switch (subtype) {
case FieldSubtype.USER:
const related = await processOutputBBReferences(
Copy link
Collaborator

Choose a reason for hiding this comment

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

One thing to consider here is performance if we have a lot of rows/links, this is inside a double loop, this await could massively delay things - I wonder if there is a better way we can manage this (like bulking the requests in someway?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As discussed, simplified to use a formula if required

@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2023

Codecov Report

Merging #12003 (6c8c0b6) into feature/multi-user-type-column (6a39eb6) will increase coverage by 18.09%.
The diff coverage is 100.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@                         Coverage Diff                         @@
##           feature/multi-user-type-column   #12003       +/-   ##
===================================================================
+ Coverage                           56.44%   74.53%   +18.09%     
===================================================================
  Files                                 259      326       +67     
  Lines                                7916    13859     +5943     
  Branches                             1471     2905     +1434     
===================================================================
+ Hits                                 4468    10330     +5862     
- Misses                               3162     3293      +131     
+ Partials                              286      236       -50     
Files Coverage Δ
...ackages/server/src/utilities/rowProcessor/index.ts 69.01% <100.00%> (ø)

... and 584 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@adrinr adrinr force-pushed the fix/use-user-table-as-display-column branch from cec5ebd to 07b8218 Compare October 9, 2023 13:30
@adrinr adrinr marked this pull request as draft October 9, 2023 13:36
@adrinr adrinr changed the title Use user column as display column Ban user column to be used as display column Oct 9, 2023
@adrinr adrinr marked this pull request as ready for review October 9, 2023 13:49
@adrinr adrinr requested a review from mike12345567 October 9, 2023 13:49
Copy link
Collaborator

@mike12345567 mike12345567 left a comment

Choose a reason for hiding this comment

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

LGTM!

@adrinr adrinr merged commit 6987d42 into feature/multi-user-type-column Oct 9, 2023
@adrinr adrinr deleted the fix/use-user-table-as-display-column branch October 9, 2023 13:52
@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants