trailblazer / reform

Form objects decoupled from models.
https://trailblazer.to/2.1/docs/reform.html
MIT License
2.49k stars 184 forks source link

Validation block option :form incorrectly memoized between tests (in different form instances?) #511

Closed AndKiel closed 4 years ago

AndKiel commented 4 years ago

Complete Description of Issue

I'm trying to replace ActiveModel validations with dry-validation. I have a pretty simple form that's supposed to validate that name is present and unique in the scope of a relation. My tests which worked with ActiveModel validations are currently failing because somehow model inside the validation block is different from the one I'm passing.

Steps to reproduce

Branch in my project: https://github.com/AndKiel/tournaments-api/tree/55eaf06f5e45e3969b0bc2720306d91e64aea6a3

Relevant files:

Test run focused on the form spec: https://github.com/AndKiel/tournaments-api/pull/29/checks?check_run_id=728156107

Expected behavior

The correct model should be present inside the form during validation so it can be used via option in the validation block.

Actual behavior

Model from the first test that ran is somehow memoized and the same in all the tests. Doesn't matter if I use subject or do CompetitorForm.new(competitor) in each test.

System configuration

Reform version: v2.3.1 (latest) dry-validation version: v1.5 (latest)

skcc321 commented 4 years ago

the same problem. any updates on that?

skcc321 commented 4 years ago

@seuros it happens here https://github.com/trailblazer/reform/blob/master/lib/reform/form/dry/new_api.rb#L38 when contract is built #call does not rewrite :form.

skcc321 commented 4 years ago

@seuros any eta what it can be fixed?

seuros commented 4 years ago

Not yet.
I dropped the legacy dry api in a new branch to locate the error. Another error is that on some test, the dry-schema lose it messages object.

skcc321 commented 4 years ago

@seuros what if we build @validator on each :call and do not memoize it at all?

Retttro commented 4 years ago

Same issue for me. @seuros Are there any updates?

seuros commented 4 years ago

@skcc321 , we could do that. Can you open a PR againt the dropDry branch ? @Retttro , got sucked into other upgrades . Can you help with a PR ?

emaglio commented 4 years ago

@AndKiel are you able to test this branch https://github.com/trailblazer/reform/tree/emaglio/drop-dry-compatibility-before-1 for your code/PR?

emaglio commented 4 years ago

@skcc321 @Retttro can you guys try this branch https://github.com/trailblazer/reform/tree/emaglio/drop-dry-compatibility-before-1 please?

AndKiel commented 4 years ago

@emaglio I completely removed reform from my project a month ago and currently use dry-validation on its own.

Retttro commented 4 years ago

@emaglio It worked for me, all tests are now green. Looking forward to a new release with the fix.

seuros commented 4 years ago

I will release it now.