messenger-monitor-bundle icon indicating copy to clipboard operation
messenger-monitor-bundle copied to clipboard

Add input data to processed messages

Open SpartakusMd opened this issue 2 years ago • 14 comments

Fixes #17

  • Uses symfony/serializer to normalize the message from the envelope.
  • Stores the data into the input field
  • Didn't work on tests yet, until it's confirmed the approach is good
image

SpartakusMd avatar Nov 12 '23 11:11 SpartakusMd

Thanks for the PR @SpartakusMd! I'm quite swamped at the moment so I'll give it a proper review (and start working on the other issues) when I have a chance.

Curious about what you said in the issue:

It gives the possibility to process the data later

Using symfony/serializer makes this difficult to do, right? If this is a valid use case, I wonder if we shouldn't use messenger's serializer. Then, when the user wants to re-run the message, we should just be able to unserialize and send to the bus. To keep the info available in the UI, we'd unserialize the data (using the messenger serializer), then normalize (using symfony/serializer). I hope that makes sense...?

kbond avatar Nov 14 '23 16:11 kbond

Thanks for the PR. Look already promising. And of course a rerun option would be great.

Chris53897 avatar Nov 23 '23 08:11 Chris53897

The input appearance on details page: Screenshot 2024-08-12 172731

aborza avatar Aug 12 '24 15:08 aborza

@kbond what do you think about the implementation? We're using the fork now in production and improves a lot the debugging life.

SpartakusMd avatar Aug 22 '24 07:08 SpartakusMd

Looks good, I'm going to be adding this bundle to an ap so I'll be able to test this out locally soon.

kbond avatar Aug 22 '24 16:08 kbond

@kbond I found an issue related to emails. If the email is sent via Messenger, it will also be captured and save to the database. The problem comes when the message contains attachments, they will also be serialized and saved to the database. I propose to clone the message before serializing and remove attachments from it. What do you think?

SpartakusMd avatar Aug 23 '24 07:08 SpartakusMd

In the end, I have added a new stamp DisableInputStoreStamp to disable saving the input.

SpartakusMd avatar Aug 23 '24 09:08 SpartakusMd

Hello @kbond, did you have time to check the changes?

SpartakusMd avatar Oct 16 '24 07:10 SpartakusMd

Hey @SpartakusMd!

Sorry for the delay - I haven't forgotten about this!

I'm pretty sure the next pre-release is going to require a migration because of #105 so feels like a good time to add this feature.

I think I'm going to add a global config to disable/enable the result storage by default, then add EnableResultStoreStamp/DisableResultStoreStamp (to use depending on your global config). Thinking we should do the same with this feature also (global config + EnableInputStoreStamp/DisableInputStoreStamp).

Also, for the Email attachment issue, could we detect when a field is binary (the case for attachments) and just replace with (binary)?

Thoughts?

kbond avatar Nov 10 '24 07:11 kbond

Hello @kbond, sounds good the config and stamps.

Regarding email attachment, I didn't find a way to eliminate binary fields from being saved in the DB.

SpartakusMd avatar Nov 12 '24 11:11 SpartakusMd

awesome feature!

bendavies avatar Nov 28 '24 10:11 bendavies

@kbond could we move forward with this PR? I would like to add some extra functionality on top of it and avoid piling all the changes in a single PR, which would make the review harder.

SpartakusMd avatar Jan 13 '25 13:01 SpartakusMd

@kbond friendly ping

Chris53897 avatar Apr 09 '25 14:04 Chris53897

Ak, sorry all, this keeps falling down my list. I do really want to see this feature!

kbond avatar Apr 09 '25 15:04 kbond