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

Add event data integrity test #137

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

Conversation

MKodde
Copy link
Member

@MKodde MKodde commented Jun 3, 2021

The Middleware command handler tests the correct data on an Event, this however is the deserialized data that is constructed on the event. The event payload may contain additional data which in turn could be sensitive data.

This behat test is created to test the presence of certain data on the event.

The Middleware command handler tests the correct data on an Event, this
however is the deserialized data that is constructed on the event. The
event payload may contain additional data which in turn could be
sensitive data.

This behat test is created to test the presence of certain data on the
event.
@MKodde MKodde force-pushed the feature/test-event-data branch from cec2f57 to 04c0edb Compare June 3, 2021 14:25
Copy link
Member

@phavekes phavekes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not check all of the event stream for the data?


public function findLatestByEventName(string $name): array
{
$sql = 'SELECT * FROM `event_stream` WHERE type LIKE CONCAT("%", :name) ORDER BY recorded_on DESC LIMIT 1';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you only check the latest event? If we check the complete event_stream, we could prevent other mistakes.

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.

2 participants