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

format should be called for select, delete, and first queries #26

Closed
vjpr opened this issue Jun 19, 2013 · 7 comments
Closed

format should be called for select, delete, and first queries #26

vjpr opened this issue Jun 19, 2013 · 7 comments

Comments

@vjpr
Copy link

vjpr commented Jun 19, 2013

From the docs an example of format is to be able to underscorize camel cased attrs before working with the db.

However, format is only run on insert and update. Format should be run for all sync methods.

Use Case

I use camel cased variables all through my code base, but I prefer to use underscores in my database because with Postgres, identifiers with capitalizations must be quoted which is irritating when sending raw queries to the db (e.g. when using psql on the command line).

@tgriesser
Copy link
Member

First makes sense, since that's based off the model attr's... what would the format be used on with the select and delete query though?

@vjpr
Copy link
Author

vjpr commented Jun 19, 2013

@forge
      name: profile.displayName
      fb: JSON.stringify profile
      fbId: profile.id
.save() ...

@forge({fbId: profile.id}).fetch() // runs `where fbId = $1` instead of `where fb_id = $1`

Delete would be the same if using something other than id.

@tgriesser
Copy link
Member

Currently delete only uses the id, unless you add where clauses to the query like so:

model.query('where', 'fb_id', 'username').delete()

But I could probably change it so that if you tried to delete a model and didn't have an idAttribute it used the other attrs in the object to delete... would that make sense?

@vjpr
Copy link
Author

vjpr commented Jun 19, 2013

Probably quite a rare case but sounds good.

Just clarifying from my last post, the main issue is select queries not running format when using fetch.

@forge({fbId: profile.id}).fetch() // runs where fbId = $1instead ofwhere fb_id = $1``

This complicates things like findOrCreate.

@tgriesser
Copy link
Member

Right... I'll change it to use format in the first fetch call for models.

@vjpr
Copy link
Author

vjpr commented Jun 19, 2013

First makes sense, since that's based off the model attr's...

...just realised what you meant - I didn't read that as the method Sync#first.

Makes sense now.

tgriesser added a commit that referenced this issue Jun 20, 2013
* master:
  0.1.9
  return undefined if model isn't fetched, fixes #21
  allow an array to be passed to hasTimestamps, fixes #25
  adding a plugin for node callback interface exec method, fixes #20
  format on the first queries, fixes #26
@tgriesser
Copy link
Member

Decided not to make the change on the delete... The reasoning that it shouldn't be easy to delete models if you accidentally left of the idAttribute value or something... for this you'll need to do

model.query('where', {some: 'value'}).del(...`

The Sync#first should be good now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants