@afterSave hook not always firing on model update

[theRealPadster]

I'm not sure if this should be a discussion or a bug report, so I figured I'd put it here first to be safe.

I'm working on a marketplace app where you can post ads. It looks like the afterSave hook I have on my PartAd model doesn't run for the one update query I make. I have a dashboard where you can pause/unpause your ads, and this works, but when I hit the same API route from my Edit Ad page, it does persist to the DB, but does not run the afterSave hook. I can tell because I added a console.debug log when all the hooks run.

I'm not sure what parts of the code are the most relevant, so please let me know if there's something that would help diagnose the issue, like which parts of the PartAd model might be relevant.

Package version

"@adonisjs/core": "^5.9.0",
"@adonisjs/lucid": "^18.3.0",

Node.js and npm version

Node 20.8.0

Sample Code (to reproduce the issue)

From edit-ad.ts (not working)

const submitUrl = stardust.route('ads.parts.update', { slug });
const response = await fetch(submitUrl, {
          method: 'PATCH',
          credentials: 'same-origin',
          body: formData,
          headers: {
            'accept': 'application/json',
          },
        });

Saving an ad logs these hooks:

=== Running hook: validateEnums(beforeSave)
=== Running hook: validateComplete(beforeSave)
=== Running hook: validateAdLimits(beforeSave)
=== Running hook: setPublishedAt(beforeSave)

And this is the formData being sent:

{
    "name": "Testing Model Application 2",
    "category": "2",
    "subcategory": "",
    "partNum": "1234 XYZ-QQ -",
    "serialNum": "2",
    "year": "2023",
    "manufacturer": "ENSTROM",
    "modelApplication": "480B TURBINE",
    "condition": "Used serviceable",
    "warranty": "Yes",
    "country": "Canada",
    "provinceState": "British Columbia",
    "description": "a",
    "contactForPrice": "on",
    "slug": "testing-model-application-2-wsTZXNOnXP",
    "_csrf": "...",
    "media": "null"
}

From dashboard.ts (working)

const url = stardust.route('ads.parts.update', { slug: card.dataset.slug });
      const response = await fetch(url, {
        method: 'PATCH',
        credentials: 'same-origin',
        body: formData,
        headers: {
          'accept': 'application/json',
        },
      });

Pausing/unpausing the ad logs these hooks:

=== Running hook: validateEnums(beforeSave)
=== Running hook: validateComplete(beforeSave)
=== Running hook: validateAdLimits(beforeSave)
=== Running hook: setPublishedAt(beforeSave)
=== Running hook: syncToAlgolia(afterSave)

And this is the formData being sent:

{
    "_csrf": "...",
    "status": "ACTIVE"
}

From PartAdController.ts

/**
   * API route to update a resource
   * @param params.slug Slug of the ad to update
   */
  public async update({ auth, params, request, response, logger }: HttpContextContract) {
    // Parse the JSON-stringified media array, so we can validate it
    request = parseMediaInRequest(request, logger);

    const adSchema = schema.create({
      // Part fields
      name: schema.string.optional({ trim: true }),
      category: schema.number.optional(),
      subcategory: schema.number.optional(),
      partNum: schema.string.nullableAndOptional({ trim: true }),
      serialNum: schema.string.nullableAndOptional({ trim: true }),
      year: schema.number.nullableAndOptional(),
      manufacturer: schema.string.optional({ trim: true }),
      modelApplication: schema.string.nullableAndOptional({ trim: true }),
      condition: schema.string.optional({ trim: true }),
      warranty: schema.string.optional({ trim: true }),
      // PartAd fields
      description: schema.string.optional({ trim: true }),
      country: schema.string.optional({ trim: true }),
      provinceState: schema.string.optional({ trim: true }),
      // Restrict length to number(11, 2) (what the DB supports)
      price: schema.number.nullableAndOptional([
        rules.unsigned(),
        rules.range(0, 999999999.99),
      ]),
      // contactForPrice: schema.boolean.optional(), // not actually used
      status: schema.enum.optional(AD_STATUSES_ARRAY),
      isFeatured: schema.boolean.optional(),
      media: schema.array.nullableAndOptional().members(filestackUploadSchema),
    });

    // TODO: error handling
    const data = await request.validate({ schema: adSchema });

    // console.debug({ data });

    // Get the part ad
    // const partAd = await PartAd.findOrFail(params.id);
    // Change this to use the slug instead of the ID
    const partAd = await PartAd.query()
      .preload('item')
      .where('slug', params.slug)
      .where('userId', auth.user!.id) // Make sure the user owns the ad
      .firstOrFail();

    const originalStatus = partAd.status;

    // Update the part
    await partAd.item.merge({
      name: data.name,
      category_id: data.subcategory || data.category,
      partNum: data.partNum,
      serialNum: data.serialNum,
      year: data.year,
      manufacturer: data.manufacturer,
      modelApplication: data.modelApplication,
      condition: data.condition,
      warranty: data.warranty,
    }).save();

    // Update the part ad
    // I need to remove undefined fields because the PartAd.validateComplete hook will actually read them
    // and say you're missing required fields, so it'll set the ad status to DRAFT
    const partAdDataToMerge = removeUndefinedFields({
      name: data.name,
      description: data.description,
      country: data.country,
      provinceState: data.provinceState,
      price: data.price,
      status: data.status,
      isFeatured: data.isFeatured,
    });
    await partAd.merge(partAdDataToMerge).save();

    // Update the media (images and documents)
    // Query for all media. We'll need to compare the new media to the old media and respond accordingly
    const mediaInDB = await partAd.related('media').query();

    // If clearing media, remove all linked
    if (data.media === null) {
      // console.debug('Clearing media');
      await Promise.all(mediaInDB.map(async (media) => await media.delete()));
    } else if (data.media) {
      // If updating media, compare the new media to the old media
      // If the new media has a filestack handle that is not in the old media, create a new media
      // If the old media has a filestack handle that is not in the new media, delete the old media
      // If the new media has a filestack handle that is in the old media, do nothing
      // If the old media has a filestack handle that is in the new media, do nothing
      const newMediaHandles = data.media.map((media) => media.handle);
      const oldMediaHandles = mediaInDB.map((media) => media.handle);

      // Create new media
      const newMediaToCreate = data.media.filter((media) => !oldMediaHandles.includes(media.handle));
      // console.debug('newMediaToCreate', newMediaToCreate);
      await Promise.all(newMediaToCreate.map(async (filestackImg) => {
        const media = await Media.create({
          handle: filestackImg.handle,
          filename: filestackImg.filename,
          mimetype: filestackImg.mimetype,
          size: filestackImg.size,
          url: filestackImg.url,
          // order: 0, // Leaving out order since it's all calculated at the end
        });
        await partAd.related('media').save(media);
      }));

      // Delete old media
      const oldMediaToDelete = mediaInDB.filter((media) => !newMediaHandles.includes(media.handle));
      // console.debug('oldMediaToDelete', oldMediaToDelete);
      await Promise.all(oldMediaToDelete.map(async (mediaItem) => await mediaItem.delete()));

      // If we did anything, update the media order
      if (newMediaToCreate.length > 0 || oldMediaToDelete.length > 0) {
        // Update the order for what's in the DB
        // Go through all the new media from the request and update the order field on the matching DB media
        const mediaFreshFromDB = await partAd.related('media').query();
        // console.debug('mediaFreshFromDB', mediaFreshFromDB);
        await Promise.all(data.media.map(async (media, index) => {
          const mediaToUpdate = mediaFreshFromDB.find((m) => m.handle === media.handle);
          if (mediaToUpdate) {
            mediaToUpdate.order = index;
            await mediaToUpdate.save();
          }
        }));
      }
    }

    return response
      .status(200)
      .json({
        success: true,
        message: 'part ad updated',
        // Send if the ad is now ready to publish (if it was previously a draft and is now inactive)
        // This is used on the edit page to redirect to dashboard with a modal or not
        readyToPublish: originalStatus === AdStatuses.DRAFT && partAd.status === AdStatuses.INACTIVE,
      });
  }

[theRealPadster]

After some more testing, I have a hypothesis. It looks like if I just update the "manufacturer" field on the linked Part model, it fires the PartAd beforeSave hooks, but not the PartAd afterSave hooks. Would I be correct in assuming that if the model does not change, it does not fire the afterSave hooks even when you call model.save() manually? Since technically what I am doing in this instance is modifying a linked model and not touching the model with the hooks, just calling .save() on it.

[tomgobich]

Yes, that's correct! It checks the $dirty flag and when it's falsy it exists out and won't attempt to persist nor call the after update/save hooks.

https://github.com/adonisjs/lucid/blob/develop/src/Orm/BaseModel/index.ts#L1782-L1787

[theRealPadster]

Okay, that makes sense. I ended up fixing my problem by adding a beforeSave hook to the PartAd model that manually updates the updatedAt field, which suffices to make the afterSave hook fire. I'm not sure if there's a better solution, but it works for now.

[tomgobich]

Looks like if you're using an updated version of Lucid, you might be able to call enableForceUpdate to force the record to update.

[theRealPadster]

Ooh, that does indeed work and is much cleaner, thank you!