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

FormulaWidget: implement new custom operation mechanism #699

Merged

Conversation

zbigg
Copy link
Contributor

@zbigg zbigg commented Jun 6, 2023

Description

Shortcut: https://app.shortcut.com/cartoteam/story/320253/implement-new-custom-operation-mechanism-in-formulawidget-in-c4r

FormulaWidget support for 'custom' aggregation expression passed as operationExp and new onStateChange callback
Internal: useWidgetFetch support for onStateChange callback

Type of change

  • Feature

Acceptance

N/A

Basic checklist

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

@shortcut-integration
Copy link

@coveralls
Copy link
Collaborator

coveralls commented Jun 6, 2023

Pull Request Test Coverage Report for Build 5389020875

  • 23 of 25 (92.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.08%) to 71.513%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/react-widgets/src/hooks/useWidgetFetch.js 18 19 94.74%
packages/react-widgets/src/models/FormulaModel.js 2 3 66.67%
Totals Coverage Status
Change from base Build 5378433190: 0.08%
Covered Lines: 2242
Relevant Lines: 2904

💛 - Coveralls

@zbigg zbigg force-pushed the feature/sc-320253/implement-new-custom-operation-mechanism branch from bab0af3 to 23c9607 Compare June 21, 2023 14:44
@github-actions
Copy link

github-actions bot commented Jun 21, 2023

Visit the preview URL for this PR (updated for commit 96edcda):

https://cartodb-fb-storybook-react-dev--pr699-feature-sc-32025-nu2uo8ng.web.app

(expires Tue, 04 Jul 2023 11:15:16 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 517cc4d31d7e09cf277774e034094b67c301cd4c

@zbigg zbigg marked this pull request as ready for review June 21, 2023 14:55
@zbigg zbigg requested review from stefano-xy, VictorVelarde and Josmorsot and removed request for stefano-xy June 21, 2023 14:57
CHANGELOG.md Outdated
@@ -2,6 +2,9 @@

## Not released

- Formula widget supports custom aggregation expression [699](https://github.com/CartoDB/carto-react/pull/699)
- Fix executeModel get url calculation [699](https://github.com/CartoDB/carto-react/pull/699)
Copy link
Contributor

Choose a reason for hiding this comment

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

fix... affecting what part of widgets? all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it's for all remote-mode & global widgets

@@ -95,15 +103,17 @@ export default function useWidgetFetch(
spatialFilter: geometryToIntersect
})
.then((data) => {
onStateChange?.({ state: WidgetStateType.Success, data });
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice and convenient

Copy link
Contributor

@VictorVelarde VictorVelarde left a comment

Choose a reason for hiding this comment

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

LGTM @zbigg

@zbigg zbigg force-pushed the feature/sc-320253/implement-new-custom-operation-mechanism branch 3 times, most recently from e9f8f90 to f876afa Compare June 23, 2023 09:21
@zbigg zbigg force-pushed the feature/sc-320253/implement-new-custom-operation-mechanism branch from f876afa to e76ac3d Compare June 23, 2023 11:20
@Josmorsot Josmorsot merged commit fbbbb1f into master Jun 28, 2023
@Josmorsot Josmorsot deleted the feature/sc-320253/implement-new-custom-operation-mechanism branch June 28, 2023 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants