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

Check null value to avoid crash on category widget #439

Merged

Conversation

moicalcob
Copy link
Contributor

@moicalcob moicalcob commented Jun 27, 2022

Description

Shortcut: https://app.shortcut.com/cartoteam/story/239960/ac-pbwqze2p-carto-3-user-s-category-widget-breaking-on-search

We've encountered that there is an issue with category widgets. The whole application crashes if we use this widget filtering a column that contains null values.

Type of change

  • Fix

Acceptance

  1. Add a category widget to a map created filtering a column that contains null values
  2. Check that it doesn't fail if we try to search a value in the category widget

Basic checklist

  • Good PR name
  • Shortcut link
  • Changelog entry
  • Just one issue per PR
  • GitHub labels
  • Proper status & reviewers
  • Tests
  • Documentation

@moicalcob moicalcob added bug Something isn't working widgets labels Jun 27, 2022
@moicalcob moicalcob requested a review from a team June 27, 2022 15:18
@shortcut-integration
Copy link

@moicalcob moicalcob self-assigned this Jun 27, 2022
@padawannn padawannn requested a review from Clebal June 27, 2022 16:01
Comment on lines 274 to 275
elem.name !== null &&
(elem.name?.toLowerCase().indexOf(searchValue.toLowerCase()) !== -1 ||
Copy link
Contributor

Choose a reason for hiding this comment

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

I would include also !== undefined and by that way you can avoid the optional chaining after name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! 😄

@coveralls
Copy link
Collaborator

coveralls commented Jun 27, 2022

Pull Request Test Coverage Report for Build 2575083100

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 72.213%

Totals Coverage Status
Change from base Build 2567987760: 0.02%
Covered Lines: 1602
Relevant Lines: 2077

💛 - Coveralls

@moicalcob moicalcob merged commit 8fb5ac1 into master Jun 28, 2022
@moicalcob moicalcob deleted the bug/sc-239960/ac-pbwqze2p-carto-3-user-s-category-widget branch June 28, 2022 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working widgets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants