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

[feature request]: throwIfEmpty #25

Open
ashleygwilliams opened this issue Aug 14, 2017 · 0 comments
Open

[feature request]: throwIfEmpty #25

ashleygwilliams opened this issue Aug 14, 2017 · 0 comments

Comments

@ashleygwilliams
Copy link
Contributor

ashleygwilliams commented Aug 14, 2017

this is a feature request for an option to pass to filter(). i am open to there being a completely different solution to this problem tho, so i will spend most of my time explaining the problem to be solved.

the problem

consider the following code:

function deleteBannedEmail (req, context) {
   const email = context.get('email')

  return BannedEmail.objects.get({
     'deleted:isNull': true,
    email
  }).then(email => {
    return BannedEmail.objects.filter({id: email.id}).delete().return()
  }).catch(BannedEmail.objects.NotFound, rethrow(404))
}

in order to throw a NotFound error, this code uses the get method to retrieve the object to be deleted. however, then, in order to actually delete the object, this function re-queries the db using the filter method which delete is then called on.

to avoid an extra query, but still throw if the object is empty, i use filter, then update, and then finally check the count returned by update, in code that looks like this:

function deleteBannedEmail (req, context) {
  const email = context.get('email')

  return BannedEmail.objects.filter({
    'deleted:isNull': true,
    email
  }).update({
    deleted: new Date()
  }).then((count) => {
    if (Number(count) < 1) {
      throw new reply.NotFoundError()
    }
    return reply.empty()
  })
}

proposed solution

checking this count seems a lil icky to me. therefore i propose a throwIfEmpty option on filter which will throw a NotFound error if filter returns no objects. this way, i don't have to check the count. the resulting code would look like this:

function deleteBannedEmail (req, context) {
  const email = context.get('email')

  return BannedEmail.objects.filter({
    'deleted:isNull': true,
    email
  }, { throwIfEmpty: true }).update({
    deleted: new Date()
  }).then(() => {
    return reply.empty()
  }).catch(BannedEmail.objects.NotFound, rethrow(404))
}

as i said, super open to other solutions! this is just the one i thought of. this is a pretty common use case, so conventionalizing an ergonomic way to accomplish this would be great. thanks!

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

1 participant