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

Alter email to base64, because the regex enconding #3705

Merged
merged 4 commits into from
Aug 2, 2022

Conversation

deividMatos
Copy link
Contributor

@deividMatos deividMatos commented Jul 26, 2022

Proposed changes

Changed the user email to base64 for the k8s labels objects, regarding to this issue. #3634
This was implemented based on this comment for PR #3648.

Types of changes

What types of changes does your code introduce to Litmus? Put an x in the boxes that apply

  • New feature (non-breaking change which adds functionality)
  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices applies)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc
  • I have signed the commit for DCO to be passed.
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added necessary documentation (if appropriate)

Dependency

  • Please add the links to the dependent PR need to be merged before this (if any).

Special notes for your reviewer:

@codesniffy codesniffy bot added the size/S label Jul 26, 2022
Signed-off-by: Deivid <deivid.dgm@gmail.com>
Signed-off-by: Deivid <deivid.dgm@gmail.com>

Signed-off-by: Deivid <deivid.dgm@gmail.com>
@imrajdas
Copy link
Member

Thanks, @deividMatos for this PR.

One quest- Have you tested it in your local dev?

@imrajdas imrajdas requested review from amityt and Jonsy13 July 31, 2022 19:58
@deividMatos
Copy link
Contributor Author

Thanks, @deividMatos for this PR.

One quest- Have you tested it in your local dev?

Yes, I tested it with a member of my squad and we created several flows to test in different scenarios, all in local dev

@imrajdas imrajdas merged commit c9e229b into litmuschaos:master Aug 2, 2022
@amityt
Copy link
Contributor

amityt commented Aug 3, 2022

Thanks @deividMatos for this PR! This will unblock a lot of users! Have test the changes and will take it as part of our next release! 🚀

@ludovic-pourrat
Copy link

Thanks @deividMatos works for us too !

@ludovic-pourrat
Copy link

Hi,

Could we decode this in the UI to display the originating user email ?

@amityt
Copy link
Contributor

amityt commented Aug 17, 2022

Hi @ludovic-pourrat , this is already handled in the UI. Attaching a screenshot for reference!

Screenshot 2022-08-17 at 6 56 32 PM

@ludovic-pourrat
Copy link

@amityt my bad my graphql-server was not updated, after applying the patch on my deployment, many thanks for your answer !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants