Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

[ad-hoc] [1] Add validation to makeFilterVolume #571

Closed
debnil opened this issue Oct 28, 2020 · 1 comment · Fixed by #589
Closed

[ad-hoc] [1] Add validation to makeFilterVolume #571

debnil opened this issue Oct 28, 2020 · 1 comment · Fixed by #589
Assignees
Milestone

Comments

@debnil
Copy link
Contributor

debnil commented Oct 28, 2020

We want to add validation to the config (of type *VolumeFilterConfig in makeFilterVolume in plugins/volumeFilter.go. This validation should include checking that exactly one asset cap is non-nil; the mode is valid; and optional accountIDs and market IDs are both non-nil.

Once the above is added, a test case should be added to TestMakeFilterVolume within volumeFilter_test.go. This test case should confirm that if an invalid configuration is constructed, an error is thrown by makeFilterVolume.

@debnil debnil added this to the v1.10.1 milestone Oct 28, 2020
@debnil debnil self-assigned this Oct 28, 2020
@nikhilsaraf nikhilsaraf changed the title Add validation to makeFilterVolume [1] Add validation to makeFilterVolume Nov 9, 2020
@nikhilsaraf nikhilsaraf changed the title [1] Add validation to makeFilterVolume [ad-hoc] [1] Add validation to makeFilterVolume Nov 9, 2020
@debnil
Copy link
Contributor Author

debnil commented Dec 4, 2020

The requirements for this have been amended - market IDs and account IDs can be nil, and the action must be valid. So we only need to validate that exactly one cap is non-nil, and the mode and action are valid.

nikhilsaraf pushed a commit that referenced this issue Dec 8, 2020
* Initial commit

* Vary actions and modes, check mods

* Handle nil market and account IDs in other tests

* Remove mode test

* Revert nil checks for market and account ids.

* Clarify nil ids elsewhere in codebase
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 a pull request may close this issue.

1 participant