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

Negative interface matching? #41

Closed
Anachron opened this issue Jan 7, 2016 · 13 comments
Closed

Negative interface matching? #41

Anachron opened this issue Jan 7, 2016 · 13 comments

Comments

@Anachron
Copy link

Anachron commented Jan 7, 2016

So instead of defining [id, username, created_at] as interface wouldn't it be better to do allow for the opposite and only define the things to hide?

I propose thse following syntax: ['!password'] will hide the password and use all default fields. This should also work in cascade relationships like [{user: ['!password']}].

Now I don't need to worry to keep updating the interfaces everywhere when I created or remove a field from my model.

@keithwhor
Copy link
Owner

Hm, what if somebody does something like, ['username', '!password']? I'm not opposed to the idea, but would like to consider some syntax alternatives.

@intabulas
Copy link
Contributor

What about an option to specify default exclusions on the model itself, it would remove some of the interface copy pasta when doing joins. Obviously that still leaves syntax about when you do want to include it

@keithwhor
Copy link
Owner

So my initial implementation had developers defining "default interfaces" on the Model, which was default inclusions.

I removed it because it's combining view logic with model logic and it becomes a pain in the ass to trace down / is easy to forget about. For now, I'd prefer to focus on a syntax just for excluded fields using arrInterface in Model#toObject(). We can expand the use case from there if we need to, and perhaps tie something to the Model (so you don't repeat yourself everywhere), but that would be step 2.

@intabulas
Copy link
Contributor

Yeah thats a really good point about model/view logic. Personally I don't mind the current implementation, I just don't like the copy pasta in controller methods. Why not have, as you suggest, arrInterface be a pure exclusion list. Seems a less confusing (and less typing) start.

My only thought is would that be obvious to people. For example using the twitter/user stuff from the videos, would people understand this shows everything but password from both models

this.respond(err || models, [{ user: ['password']}]);

@keithwhor
Copy link
Owner

I do not like the idea of the interface being exclusionary instead of inclusionary. It should be inclusionary by default, it's easier to reason about. Perhaps Model#toObject() takes a second parameter, than when true, turns the interface to exclusionary.

Does that sound reasonable? Then we can just add another parameter to Controller#respond, etc.

@intabulas
Copy link
Contributor

Works for me

@keithwhor
Copy link
Owner

Alright. I can do this --- a couple of other Model changes I want to make anyway. Will get to it this weekend.

@Anachron
Copy link
Author

Anachron commented Jan 7, 2016

Yes I'm also fine with that one! 👍

Sorry for my typos, I'm writing from mobile :)

@Anachron
Copy link
Author

Anachron commented Jan 7, 2016

New idea: Why not responseWithout as new method (as opposite to response)? Could that be less confusing instead of a flag?

@keithwhor
Copy link
Owner

I'd rather just have a flag. "respondWithout" doesn't mean anything. Respond without what? Chances are a new dev would look up an API reference either way.

@Anachron
Copy link
Author

Anachron commented Jan 7, 2016

Fair enough. Was just an idea.

@keithwhor
Copy link
Owner

Supported:
0031b10

Must specify: {exclude: true} in options for Model#toObject.

Not linked up to .respond method yet.

@keithwhor
Copy link
Owner

Finished with fd65291

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

No branches or pull requests

3 participants