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.

Sign up for GitHub

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

Jump to bottom

Array Validators & Transforms #29

Merged
merged 9 commits into from Oct 21, 2019
Merged

Array Validators & Transforms #29

merged 9 commits into from Oct 21, 2019

Conversation

hasezoey
Copy link
Member

@hasezoey hasezoey commented Sep 10, 2019 •
edited

(move from original repo to this fork)

Events happened since the original PR:

  • mongoose@5.7.0 released
  • rebased onto r6/master
  • upgraded to v6.0.0 "syntax"
    Update 1: - fixed it now? but it is really hacky and not future proof...

so more or less all tests except 2 pass:

This PR might be a "breaking" change

@hasezoey hasezoey added bug Something isn't working enhancement Improve an existing Feature cant fix This cant be fixed without external modules being updated breaking This PR is a breaking change | This Issue to fix would be a breaking change labels Sep 10, 2019
@hasezoey hasezoey added this to the 6.0.0 milestone Sep 10, 2019
@hasezoey hasezoey self-assigned this Sep 10, 2019
@hasezoey hasezoey added info needed Extra information is needed and removed cant fix This cant be fixed without external modules being updated labels Sep 13, 2019
@hasezoey hasezoey marked this pull request as ready for review September 13, 2019 08:06
@hasezoey
Copy link
Member Author

The Array Validators now work, but is a bit hacky & not future proof, and might be even wrong

thats why i wait for Automattic/mongoose#8012 (comment) to get a response

@hasezoey
Copy link
Member Author

@fabioformosa & @B-Stefan if you both would have the time, could you two test it if it breaks something?

@hasezoey
Copy link
Member Author

I will move this PR / feature / parity to an 6.x (unkown yet), because of the hackyness

@hasezoey hasezoey modified the milestones: 6.0.0, 6.x Sep 14, 2019
@hasezoey hasezoey mentioned this pull request Sep 16, 2019
Closed
14 tasks
@hasezoey hasezoey added parity Feature Parity with mongoose | mongodb dont merge yet Dont merge this PR *yet* and removed bug Something isn't working labels Sep 22, 2019
@B-Stefan
Copy link
Contributor

It seems that this could be a better way to support this feature #25

@hasezoey
Copy link
Member Author

hasezoey commented Sep 23, 2019 •
edited

@B-Stefan do you mean this(#29) or #25 is better?

Update:
when i see it right, #25 is about discriminators, but this (pr) is about the array-transfrom / validate options, which now work, but in a hacky way ... as far as i can see, this has nothing todo with discriminators

@B-Stefan
Copy link
Contributor

B-Stefan commented Sep 23, 2019 •
edited by hasezoey

your approach #29! :)
If this feature is complete I would be happy to contribute the discriminator feature for arrays.

@hasezoey
Copy link
Member Author

Update from mongoose issue: as by his comment, it seems like this approach i currently use is the only option, i still feel like this is hacky, but i think its not gonna change, so i will merge it

For anyone who find missing / wrong associated options, please report via a new issue, thanks

@hasezoey hasezoey mentioned this pull request Sep 30, 2019
Closed
@hasezoey hasezoey mentioned this pull request Oct 12, 2019
Closed
@hasezoey hasezoey modified the milestones: 6.x, 6.1.0 Oct 16, 2019
@hasezoey
Add SemVer to check mongoose version
8f47131 
- logger cleanup
- add semver as package to check mongoose version
@hasezoey
Copy link
Member Author

hasezoey commented Oct 17, 2019 •
edited

Current State:
Everything is working, thanks to Automattic/mongoose#8012

BUT 2 tests still fail, because 2 options are missing in mongoose, minlength and maxlength for Schema.Types.String, stated in this comment and this new issue

otherwise, cleanup would be needed

@hasezoey hasezoey changed the base branch from r6/master to master October 17, 2019 22:33
@hasezoey
Copy link
Member Author

The last remaining issue in mongoose got solved, so now every test is green
And cleanup is done

@hasezoey hasezoey removed dont merge yet Dont merge this PR *yet* info needed Extra information is needed labels Oct 21, 2019
@hasezoey
Add Documentation about "innerOptions" & "outerOptions"
daac0e0 
- arrayProp(gh-page): add "innerOptions" & "outerOptions"
- changelog(gh-page): add note that "Array Validators" are now integrated
- utils(src): actually allow the use of "innerOptions" & "outerOptions"
@hasezoey
Copy link
Member Author

As a note:

the semver check for mongoose "5.7.6" is made, because array validators only work on 5.7.5, and in version 5.7.6 the missing options were added

@hasezoey hasezoey merged commit 7529d0d into master Oct 21, 2019
@hasezoey hasezoey deleted the r6/arrayvalidators branch October 21, 2019 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@hasezoey hasezoey

Labels
breaking This PR is a breaking change | This Issue to fix would be a breaking change enhancement Improve an existing Feature parity Feature Parity with mongoose | mongodb
Projects
None yet
Milestone
6.1
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@hasezoey @B-Stefan