🐛 [kobotoolbox] Fix query,sort + use question name in attachments

[Yann-J]

Follow-up to #2765 to add some missing parameters, and use the form's corresponding question name as attachment name.

[Yann-J]

I believe that's all @RicardoE105
This adds a "Query Options" collection to the submission:getAll menu:

As well as uses the question name corresponding to each attachment, if no prefix is provided:

[Yann-J]

Hey @RicardoE105 quick ping on this one, and apologies to pressure you... It should be fairly straightforward to validate since these are small improvements over #2765...

I think the attachment names is maybe less crucial, but the ability to query for form submissions is very much needed to make this node fully useful.

[RicardoE105]

@Yann-J Sorry for the late response, I missed the first comment.

Hmm it looks like my filter and sort options from my original PR were dropped from the submission:getAll operation, this seriously limits the usefulness of the node because I believe it will be extremely rare that users will want to retrieve all submissions in a call (which is why I felt that query was a better name than getAll - though I understand the objective of consistency) - typical surveys will have 10s of thousands of submissions (ours have millions).

If I remember correctly, the filter was not working properly and even if it did, it should have been with the pattern the Notion node has, where you can build the filter either manually with a fixedCollection or provide a JSON. Assuming that is too much work, then you can only add the option to filter with JSON.

Also, I'm wondering about the attachment naming. My PR had logic to name the files after the corresponding question in the survey, which I think will be the most common case (and is guaranteed not to have conflicts). It looks like this was also dropped. This is probably less crucial, but this is certainly less convenient that way... Shall I add this back in as well?

Adding this back would not be a breaking change? I believe it will. With the approach that I followed you will not have conflicts either. We use this pattern in other nodes as well.

[Yann-J]

Thanks for getting back @RicardoE105 .

The filter definitely works (one is applied in the above screenshot for instance... if it wasn't working, that form would return ~300k submissions...) - but crafting the JSON predicate is not necessarily trivial as it's a MongoDB query passed directly to the engine - not very much we can do about this, as this is how the API works. The sample provided in the description, {"_submission_time":{"$lt":"2021-10-01T01:02:03"}} is a basic working example.
A fixedCollection may not work very well here because the full list of fields is not known in advance, and implementing the full complexity of MongoDB operators might be significant work. I would agree that some very common filters such as submission date and validation status could make sense here, but I don't think that's very urgent...

The current logic is indeed to pass the raw JSON string. Is there anything that you'd expect me to change in my PR?

Concerning the attachment naming, you're right, I realize this would be a breaking change... I think it's not too late to do this, since presumably there are still very few users of this node - and even fewer might depend on attachment namings. I do believe it makes sense to use my proposed logic as the default behavior, since this is what users would likely expect, but I suppose we can also add a new boolean to "Name attachments after their related question". Let me know what you'd suggest here...

The objective here is not to avoid naming conflicts, but to make it easy to identify the correct attachment, since those are always linked to a specific form question (even though the API does not make that link very clear - the value for these form questions will be the attachment's filename). With the current naming logic, the relationship is lost.

For example, one of our forms is used to collect farmer's picture and national ID. These are 2 separate form questions of type image with unique names like picture and idcard. In an n8n workflow, to properly identify these 2 different attachments, using the question name is much more explicit than attachment_0 and attachment_1. I am not even certain if the order is guaranteed in the API's attachments array, and if it is, it would require the user to know the order of the questions in the original form (which cannot be guessed from the submissions API response), and will likely change if the questions are reordered in the form.

[RicardoE105]

The current logic is indeed to pass the raw JSON string. Is there anything that you'd expect me to change in my PR?

It's ok if it's only a JSON string for the moment. Just do it like in the Notion example and only enable the JSON and none option.

Concerning the attachment naming, you're right, I realize this would be a breaking change... I think it's not too late to do this, since presumably there are still very few users of this node - and even fewer might depend on attachment namings. I do believe it makes sense to use my proposed logic as the default behavior, since this is what users would likely expect, but I suppose we can also add a new boolean to "Name attachments after their related question". Let me know what you'd suggest here...

The objective here is not to avoid naming conflicts, but to make it easy to identify the correct attachment, since those are always linked to a specific form question (even though the API does not make that link very clear - the value for these form questions will be the attachment's filename). With the current naming logic, the relationship is lost.

For example, one of our forms is used to collect farmer's picture and national ID. These are 2 separate form questions of type image with unique names like picture and idcard. In an n8n workflow, to properly identify these 2 different attachments, using the question name is much more explicit than attachment_0 and attachment_1. I am not even certain if the order is guaranteed in the API's attachments array, and if it is, it would require the user to know the order of the questions in the original form (which cannot be guessed from the submissions API response), and will likely change if the questions are reordered in the form.

You can add the optional to the PR, so it does not break anything.

[Yann-J]

OK, both tasks completed @RicardoE105

See below screenshot showing the updated menu structure, with working filters, as well as a new "Attachments Naming Scheme" dropdown, with a default behaviour replicating the current strategy (i.e. numbered sequence), and a new option to use the original form question.

Default behaviour:

And with the "form question" option:

This also works on the Trigger node:

Can we please have this fairly rapidly 🙏... Without both of these options, none of the workflows we were planning to roll out can be implemented...

[Yann-J]

Hey @RicardoE105 gentle nudge here...

[RicardoE105]

Will review it before the next release so we can include it.

[Yann-J]

Any update on this please @RicardoE105? ... I know you're busy, but we've been waiting on this node since December...

[RicardoE105]

@Yann-J ahh, my bad. My coworker @Joffcom will review it either today or tomorrow. I will keep you posted.

[Joffcom]

Hey @Yann-J,

Just started to look at this one, It looks like when the filter is set to None it doesn't work and throws an error as filterJson doesn't exist. I did fix it locally but your repo settings don't allow me to push the change to the branch so if you could fix that part not working the rest of it looks good.

Love the naming feature for binary data I can see why you would prefer that I know I would, The only bit that bugs me you have already talked about with @RicardoE105. It would be great if we could have a simple filter and a json filter this would make it more accessible, I couldn't find any api documentation for KoBo to see if they have a call to get the fields but that would be the place to start. I do agree it isn't urgent but it would be worth thinking about, It could be that we keep an eye on community support requests and if it pops up a lot we can spend some time implementing it.

[Yann-J]

Oops, you're right! I had done my tests with an empty filter, but not with Filter = None in the dropdown...

Thank you, well spotted!

Should be fixed now in the latest commit!

I do agree that building some basic filters with the UI would be nice. I don't believe Kobo will document this API since it seems they're just passing the filter over to MongoDB, so the API is basically the MongoDB one - which is quite exhaustive (and recursive), but in practice we probably only need to support a basic grammar. The problem is the fields, which we won't always know in advance. But for sure this can probably wait, and I'd suggest to get this one released ASAP, as I think the current version (without the filter) has limited usefulness...

Thanks for the review @Joffcom !

[Yann-J]

@Joffcom any update on this one?

[Joffcom]

Hey @Yann-J,

Initial review is complete it has been passed up and is waiting for one last review to make sure I have not missed anything then it will be merged in.

[Yann-J]

@Joffcom @RicardoE105 @janober folks, sorry to insist once again, but I see we missed the last release, and this is getting difficult for us to trust n8n to evolve fast enough for our needs.

My original PR was raised last December. I understand that the initial review took some back and forth to adhere to n8n's standards in terms of menu structure, but when it was merged, 2 important features were unilaterally dropped without discussion, which in my opinion make this node significantly less useful.

This new PR is fairly straightforward and meant to restore these dropped features, which we have now been waiting for for 6 months, as none of our workflows can currently make use of the node as it is now. I've done my very best to make the PR clean and address any comment practically overnight, because my organization is really counting on this node to perform some crucial automations. I hope you can understand my frustration.

Can I please ask for a speedy review completion? 🙏

[Joffcom]

Hey @Yann-J,

It looks like this is still in the last review column internally I will pop a note on it now.

[janober]

Sorry, that it took so long. Got now merged and will be released with the next version beginning of next week.

[janober]

Got released with n8n@0.177.0

[Yann-J]

Thank you so much this will be mightily useful for us!