Skip to content

Conversation

chazwatkins
Copy link
Contributor

@chazwatkins chazwatkins commented Aug 4, 2025

Improvements

  • Add field? option to Ash.Resource.Calculation DSL
  • Exclude field?: false calculations from Resource struct and direct loading, but still usable in expressions
  • Add validation that returns Ash.Error.InvalidLoad error when loading a field?: false calculation directly
  • Update calculations docs with explanation of field? option

We could probably do the same for aggregates once this is done.

#2242

Contributor checklist

Leave anything that you believe does not apply unchecked.

  • I accept the AI Policy, or AI was not used in the creation of this PR.
  • Bug fixes include regression tests
  • Chores
  • Documentation changes
  • Features include unit/acceptance tests
  • Refactoring
  • Update dependencies

@chazwatkins chazwatkins force-pushed the feat/ref-calcs branch 5 times, most recently from 20998d2 to 16cf732 Compare August 4, 2025 04:23
@chazwatkins chazwatkins changed the title feat: Add field?: false option to Ash.Resource.Calculation DSL feat: Exclude resource calculations from resource struct with field?: false option Aug 4, 2025
@chazwatkins chazwatkins force-pushed the feat/ref-calcs branch 2 times, most recently from 12f49e6 to 3246d93 Compare August 4, 2025 14:49
zachdaniel
zachdaniel previously approved these changes Aug 4, 2025
Copy link
Contributor

@zachdaniel zachdaniel left a comment

Choose a reason for hiding this comment

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

LGTM! Lets add that test and ship.

@chazwatkins
Copy link
Contributor Author

ok, the validation is in and you now get an InvalidLoad error when you attempt to load a field?: false resource calculation.

@chazwatkins chazwatkins marked this pull request as ready for review August 4, 2025 17:41
@chazwatkins chazwatkins force-pushed the feat/ref-calcs branch 2 times, most recently from 4cbdc85 to 629f001 Compare August 4, 2025 18:06
@zachdaniel
Copy link
Contributor

Just had a thought: we may actually have a bit of an issue here 😢

Specifically, AshGraphql and AshJsonApi will need to be updated to support this, by not allowing loads for these non-loadable calculations. We'll want PRs for those done before we release this.

@chazwatkins
Copy link
Contributor Author

Just had a thought: we may actually have a bit of an issue here 😢

Specifically, AshGraphql and AshJsonApi will need to be updated to support this, by not allowing loads for these non-loadable calculations. We'll want PRs for those done before we release this.

No experience with those packages, but I'll take a look at them.

@chazwatkins
Copy link
Contributor Author

Both api libraries only expose public?: true calculations. So, I made a small change to have public_calculations and public_calculation only return back field?: true calculations.

Do you see any cases where a field?: false calculation should be exposed outside the resource?

@zachdaniel
Copy link
Contributor

Hmm...yes. It could be supported in filters exposed over GraphQL for example without being exposed as a field.

@chazwatkins chazwatkins marked this pull request as draft August 5, 2025 14:58
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.

2 participants