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

allow empty strings in shopURL #339

Merged
merged 1 commit into from
Feb 12, 2021
Merged

Conversation

Akarshit
Copy link
Contributor

Resolves #291
Impact: minor
Type: bugfix

Issue

The SimpleSchema was cleaning the object of empty string before saving it. Also the RegEx was not allowing empty strings to pass.

Solution

  1. Configured cleaning to not remove empty strings.
  2. Modified regex to allow empty string.

Breaking changes

None expected

Testing

  1. Login as Admin
  2. Enter a URL for the store logo and save it. The top left icon should update based on the URL you entered
  3. Remove the URL and hit Save, the logo should default back to the Reaction Commerce logo.

@nnnnat
Copy link
Contributor

nnnnat commented Jan 27, 2021

@Akarshit can you sign your commit and force push?

@Akarshit
Copy link
Contributor Author

Akarshit commented Jan 27, 2021 via email

@Akarshit Akarshit force-pushed the bugfix-akarshit-remove-shopurl branch from e594718 to e899dd5 Compare January 27, 2021 19:12
@Akarshit Akarshit force-pushed the bugfix-akarshit-remove-shopurl branch from e899dd5 to 020e196 Compare January 28, 2021 17:16
@Akarshit Akarshit requested a review from jrw421 January 28, 2021 17:16
Copy link
Contributor

@jrw421 jrw421 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@Akarshit Akarshit force-pushed the bugfix-akarshit-remove-shopurl branch from 020e196 to d4e9370 Compare February 12, 2021 19:11
@Akarshit
Copy link
Contributor Author

Only updated the commit message.

@Akarshit Akarshit merged commit 3637e5d into trunk Feb 12, 2021
@Akarshit Akarshit mentioned this pull request Mar 3, 2021
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.

Unable to remove ShopLogoUrl
4 participants