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

Form - Fix form node not passing on msg.topic on submit #1483

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cgjgh
Copy link
Contributor

@cgjgh cgjgh commented Nov 19, 2024

Description

Fixes msg.topic not being applied on form submits.
Not completely sure if this meets any previously agreed upon design pattern for applying topics.

Related Issue(s)

Closes #1453

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

  • Includes a DB migration? -> add the area:migration label

@joepavitt
Copy link
Collaborator

Thanks @cgjgh - this is an odd one, as the appendTopic function runs as a universal line in

try {
msg = await appendTopic(RED, widgetConfig, wNode, msg)
// pre-process the msg before send on the msg (if beforeSend is defined)
if (widgetEvents?.beforeSend && typeof widgetEvents.beforeSend === 'function') {
msg = await widgetEvents.beforeSend(msg)
}
// send the msg onwards
wNode.send(msg)

Which is part of the onAction handler in our ui-base. So the change you've submitted, whilst working, feels like it's covering another issue we have somewhere

@joepavitt joepavitt changed the title Fix form node not passing on msg.topic on submit Form - Fix form node not passing on msg.topic on submit Dec 2, 2024
@bartbutenaers
Copy link
Contributor

feels like it's covering another issue we have somewhere

@joepavitt,
Yes indeed you are correct.
I think this is what goes wrong with the example flow from @colinl:

  1. He injects a message with only a topic, not a payload. Althought this is not relevant I think, as you can see below.
  2. The msg arrives in ui_base->onInput, where the msg (containing the topic) is stored in the datastore and emmited to the frontend.
  3. The msg (containing the topic) arrives in the onInput of the form vue component, where the msg is ignored because it has no payload. But even if it would have a payload, only that payload would be processed (and the topic would be ignored anyway):
            onInput (msg) {
             if (msg.payload) {
                 const payload = msg.payload
                 for (const key in payload) {
                     this.input[key] = payload[key]
                 }
                 this.$nextTick(() => { this.validate() })
             }
         },
    
  4. The user clicks on the form "Submit" button.
  5. The onSubmit in Vue is called, which will create a new message from scratch (which only contains a payload) to the server:
    this.$socket.emit('widget-action', this.id, {
       payload: this.input
    })
    
  6. The frontend msg (only containing a payload) will arrive in the ui_base-->onAction on the server side, where it will append the topic to that msg:
    msg = await appendTopic(RED, widgetConfig, wNode, msg)
    
    However the frontend msg only contains a payload, and the widgetConfig only contains msg.topic (to tell the function that it should get the topic from the msg). So NO topic is appended to the output msg...

You can solve this in various ways, but not sure what is the best way to do it:

  • You could store the topic in the frontend, and send it back on commit. But seems a bit weird to use an entire roundtrip.
  • You could get the topic in the onAction from the last msg in the datastore (if the msg doesn't contain a topic). But the appendTopic is implemented in a separate util file, so you cannot do that in the util file. Which means you need to append the topic to the msg before you call the appendTopic function, which is also very weird.

Any thoughts are welcome!

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

Successfully merging this pull request may close these issues.

Form node does not pass on topic when configured to do so
3 participants