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.

Sign up for GitHub

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

Jump to bottom

[DIA-2151] Removed singleShotPM from ConsentActionImpl #657

Merged
merged 3 commits into from
Jul 4, 2023
Merged

[DIA-2151] Removed singleShotPM from ConsentActionImpl #657

merged 3 commits into from
Jul 4, 2023

Conversation

Nevazhnovu
Copy link
Collaborator

No description provided.

@andresilveirah
Copy link
Member

@Nevazhnovu there are other places where singleShot is being used. Did you consider removing them as as well? Also, did you check if the callbacks behave correctly for a campaign where a PM is used as 1st layer message?

@Nevazhnovu
Copy link
Collaborator Author

@andresilveirah it's used inside js_reciever.js (to communicate with backend I believe) and nowhere else in the Android SDK itself, after I removed the entrances of singleShot in this PR.
I don't think that's a good idea to remove it completely, just in the case described in the ticket (which I did)

@Nevazhnovu
Copy link
Collaborator Author

I've removed it completely and manually tested with 3 properties - it works great

@andresilveirah
Copy link
Member

@Nevazhnovu I noticed onSPFinished was not being called if the user would load the PM (not via 1st layer message) and dismiss it. I took the liberty to fix that, refactoring ClientEventManager in the process and written a few tests for it.

@andresilveirah andresilveirah force-pushed the DIA-2151_fix_onSPFinished_called_from_2nd_layer_cancel_button branch from 0fe98ef to a0eeb4f Compare June 30, 2023 11:01
@andresilveirah andresilveirah merged commit f7aec1d into develop Jul 4, 2023
3 of 4 checks passed
@andresilveirah andresilveirah deleted the DIA-2151_fix_onSPFinished_called_from_2nd_layer_cancel_button branch July 4, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andresilveirah andresilveirah andresilveirah approved these changes

@bohdan-go-wombat bohdan-go-wombat bohdan-go-wombat approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Nevazhnovu @andresilveirah @bohdan-go-wombat