Objection.js: [psql] $relatedQuery().patch().returning("*"): does not return object, debug shows returning: undefined

[the-vampiire]

what is happening

• without $beforeUpdate hook (sets updated_at ts) method on base Model class
  
  
  
  • returns the updated rows count instead of returning(*)
  
  
• with $beforeUpdate hook method
  
  
  
  • returns the Student (relation) object but without extra fields
  
  

what i intended

• update the (extra) columns in the join table
• return the Student (relation) object with the extra fields

query

// proto method, [this] is a Course instance
this.$relatedQuery("students")
      .where("student_id", studentId)
      .patch({ paymentDate: new Date().toISOString(), confirmationId })
      .returning("*")
      .first();

Course relations

students: {
        modelClass: "student",
        relation: Model.ManyToManyRelation,
        join: {
          from: "courses.id",
          to: "students.id",
          through: {
            from: "payments.course_id",
            to: "payments.student_id",
            extra: {
              amount: "amount",
              paymentType: "payment_type",
              invoiceDate: "invoice_date",
              paymentDate: "payment_date",
              confirmationId: "confirmation_id",
            },
          },
        },

base class (extended by Course, Student, Payment)

/**
 * Base class for shared configuration
 * - converts snake_case (db side) <--> camelCase (server side)
 * - sets base model path
 *  - allows relations with modelClass to use 'modelFileName' string
 * - automatic timestamps
 *  - $beforeInsert(): created_at = new Date as ISO string
 *  - $beforeUpdate(): updated_at = new Date as ISO string
 *  - call super[.$beforeInsert()][.$beforeUpdate()] if overriding in subclass
 */
class Base extends Model {
  static get columnNameMappers() {
    return snakeCaseMappers();
  }

  static get modelPaths() {
    return [__dirname];
  }

  $beforeInsert() {
    this.created_at = new Date().toISOString();
  }

// tried with and without this hook method
 $beforeUpdate() {
    this.updated_at = new Date().toISOString();
  }
}

module.exports = Base;

behavior with $beforeUpdate hook removed

• updates paymentDate and confirmationId, extra fields on the join table payments
• returns updated count instead of Student object
  
  
  
  • update count is 0 (because Student relation was not updated?)
  
  

debug output

{ method: 'update',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings:
   [ '2019-06-04T19:47:56.942Z',
     'ch_1EhibwChegPuw7tfaOXZ3Nsr',
     1,
     '13',
     1 ],
  __knexQueryUid: '7617d521-6bf1-4e14-bdfc-8e8150a7f04a',
  sql:
   'update "payments" set "payment_date" = ?, "confirmation_id" = ? where "payments"."student_id" in (select "students"."id" from "students" inner join "payments" on "students"."id" = "payments"."student_id" where "payments"."course_id" in (?) and "student_id" = ?) and "payments"."course_id" = ?',
  returning: undefined }

behavior with$beforeUpdate hook setting updated_at on Student

• updates paymentDate and confirmationId, extra fields on the join table payments
• returns the Student object but without extra fields

debug output

{ method: 'update',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings: [ '2019-06-04T19:49:22.388Z', 1, '13' ],
  __knexQueryUid: 'ca24b0c2-f5ed-4b52-9e2d-83926d711be5',
  sql:
   'update "students" set "updated_at" = ? where "students"."id" in (select "students"."id" from "students" inner join "payments" on "students"."id" = "payments"."student_id" where "payments"."course_id" in (?) and "student_id" = ?) returning *',
  returning: [ '*' ] }
{ method: 'update',
  options: {},
  timeout: false,
  cancelOnTimeout: false,
  bindings:
   [ '2019-06-04T19:49:22.344Z',
     'ch_1EhidJChegPuw7tfx18bBNn1',
     1,
     '13',
     1 ],
  __knexQueryUid: '99888899-70b2-4049-bace-9d973b524e27',
  sql:
   'update "payments" set "payment_date" = ?, "confirmation_id" = ? where "payments"."student_id" in (select "students"."id" from "students" inner join "payments" on "students"."id" = "payments"."student_id" where "payments"."course_id" in (?) and "student_id" = ?) and "payments"."course_id" = ?',
  returning: undefined }

[koskimas]

When you run a patch operation that only has extra properties, there's nothing to update in the students table. We couldn't create an update even if we wanted to. What would that update update? Therefore the students update is skipped alltogether and nothing gets returned. When you enable the hook, there is something to update (the updated_at column) and returning works. It's a pity this happens, but there is really no way around it except to create a dummy update like

update students set id = id where ...

but that causes problems when there's a constraint that prevents identifiers from updating (some databases have those). People can also easily add those kind of constraints or triggers themselves to make sure nothing gets updated. Same goes for other fields.

[the-vampiire]

@koskimas ah that makes sense man, i understand.

can you recommend a different way to patch the extra fields and get back the relation object + extra fields?

my goals is to complete all of this relative to a Course instance:

• update the extra fields (on the payments table joining courses and students)
• return a Student object with the (updated) extra fields

as a workaround i had to create a Student proto method and manually set the fields. but this mutates the object externally. i guess theres nothing wrong with it its just not the type of methods i am used to writing.

id rather i call a query that returns the student + extra fields at once instead of mutating the Student object.

student proto method

 async setPaymentConfirmation(courseId, confirmationId) {
    const paymentDate = new Date().toISOString();

    await this.$relatedQuery("payments")
      .patch({ paymentDate, confirmationId })
      .where("payments.course_id", courseId);

    // set the updated payment properties manually onto the student
    this.$set({ paymentDate, confirmationId });
  }

which is now used in the original method:

course method

static async completeStripePayment(paymentData, stripeService) {
    const { courseId, studentId } = paymentData;

    const course = await this.query()
      .findById(courseId)
      .select(["id", "price", "name"])
      .throwIfNotFound();

    // throws if not found (student not registered)
    const student = await course.getRegisteredStudent(studentId);

    if (student.paymentDate) {
      // student has already paid, exit early
      return student;
    }

    const confirmationId = await stripeService.createCharge(
      course,
      paymentData,
    );

    // calling the student proto method
    await student.setPaymentConfirmation(courseId, confirmationId);

    return student;
  }

[koskimas]

Does this work?

this.$relatedQuery("students")
      .patchAndFetchById(studentId, { 
        paymentDate: new Date().toISOString(), 
        confirmationId 
      })

It probably should work (and return the extra props too). If this doesn't work, I'll see if I can easily fix it.

[the-vampiire]

@koskimas hell ya it did! thanks a lot man. i completely overlooked patchAndFetchById. i had tried updateAndFetchById but it (as expected) failed schema validation since i was only setting partial data.

thanks a lot for the help and patience man. im coming from a long time usage of sequelize and objection / knex has been an absolute breath.

[the-vampiire]

Does this work?

this.$relatedQuery("students")
      .patchAndFetchById(studentId, { 
        paymentDate: new Date().toISOString(), 
        confirmationId 
      })

It probably should work (and return the extra props too). If this doesn't work, I'll see if I can easily fix it.

hey @koskimas i was debugging some weird behavior and realized it led back to this query. it turns out that this query will apply the patch to all the payments (not just that of the student by its studentId). so my tests passed when i had one student but with multiple students it fails (due to a unique constraint error on confirmationId)

two questions

• is this the expected behavior? i thought given a studentId it would use that to narrow the scope of the patch but as seen in the log below it does not
• do you have any suggestion on accomplishing this behavior (patch the join table, payments, and return the many side, students) but narrowing the patch only to the particular student?

logs

console.log node_modules/knex/lib/logger.js:16
    { method: 'update',
      options: {},
      timeout: false,
      cancelOnTimeout: false,
      bindings: [ 'CREDIT', '2019-08-04T00:00:34.604Z', 'tity', 374, 374 ],
      __knexQueryUid: 'db9966e9-64ed-42f0-965e-a3fe8bc22d0c',
      sql:
       'update "payments" set "payment_type" = ?, "payment_date" = ?, "confirmation_id" = ? where "payments"."student_id" in (select "students"."id" from "students" inner join "payments" on "students"."id" = "payments"."student_id" where "payments"."course_id" in (?)) and "payments"."course_id" = ?',
      returning: undefined }

  console.log node_modules/knex/lib/logger.js:16
    { method: 'select',
      options: {},
      timeout: false,
      cancelOnTimeout: false,
      bindings: [ 411 ],
      __knexQueryUid: '0eb5f4b0-daa6-4d8a-8073-1f131e76f151',
      sql:
       'select "students".* from "students" where "students"."id" = ?' }

[the-vampiire]

this is the part of the query that applies the patch to all the payments rows instead of just that which matches studentId and courseId

where "payments"."student_id"
  in (
    select "students"."id" from "students" inner join "payments" 
    on "students"."id" = "payments"."student_id" where "payments"."course_id" in (?)
  )
  and "payments"."course_id" = ?'

[koskimas]

Looks like a bug.

[koskimas]

Finally fixed. Sorry it took so long. A new version containing this fix will be released shortly.