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

Chore: Fix Omnichannel E2E tests not running #26092

Merged
merged 9 commits into from
Jul 19, 2022
Merged

Chore: Fix Omnichannel E2E tests not running #26092

merged 9 commits into from
Jul 19, 2022

Conversation

murtaza98
Copy link
Contributor

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

@murtaza98 murtaza98 requested a review from a team as a code owner July 1, 2022 15:53
@murtaza98 murtaza98 requested a review from KevLehman July 1, 2022 15:53
Comment on lines 17 to 18
'tests/end-to-end/api/livechat/*.js',
'tests/end-to-end/api/livechat/*.ts',
Copy link
Member

Choose a reason for hiding this comment

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

I think it is better to use a greedy glob, something like:

		'tests/end-to-end/api/**/*.js',
		'tests/end-to-end/api/**/*.ts',

so it is not needed to have specific lines for livechat and whatever else we add

@@ -287,7 +287,7 @@ export class LivechatVisitorsRaw extends BaseRaw<ILivechatVisitor> implements IL
}

removeById(_id: string): Promise<DeleteResult> {
return this.removeById(_id);
return this.deleteOne({ _id });
Copy link
Member

@sampaiodiego sampaiodiego Jul 19, 2022

Choose a reason for hiding this comment

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

is this being used? I mean, glad you fixed since it is an infinite recursive loop, if this is being used somewhere how none found it before? 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I believe this is getting used on Livechat when visitor requests to delete their data 🙈

Copy link
Contributor

Choose a reason for hiding this comment

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

It's used as murtaza said. But as that feature is not that used 🙈 probably no one found it. (This single line could easily be a regression tho)

@sampaiodiego sampaiodiego merged commit 35b9384 into develop Jul 19, 2022
@sampaiodiego sampaiodiego deleted the omni/tests branch July 19, 2022 16:15
@murtaza98 murtaza98 mentioned this pull request Jul 21, 2022
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.

4 participants