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

2006 trigger error on url in xform title #2007

Merged
merged 2 commits into from
Feb 5, 2021

Conversation

ivermac
Copy link
Contributor

@ivermac ivermac commented Jan 29, 2021

Changes / Features implemented

Trigger error when xform title matches a url

Steps taken to verify this change does what is intended

Added a test to check that an error is triggered when xform title matches a url

Side effects of implementing this change

Closes #2006

@ivermac ivermac force-pushed the 2006-trigger-error-on-url-in-xform-title branch 2 times, most recently from e28c0ae to e7c43c4 Compare January 29, 2021 15:35
DavisRayM
DavisRayM previously approved these changes Feb 1, 2021
Copy link
Contributor

@DavisRayM DavisRayM left a comment

Choose a reason for hiding this comment

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

LGTM, just a few suggestions


if re.match(pattern, self.title):
raise XLSFormError(
_("Invalid title value; shouldn't matches a url"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_("Invalid title value; shouldn't matches a url"))
_("Invalid title value; value shouldn't match a URL"))

@@ -818,6 +818,13 @@ def _set_title(self):
_("Title shouldn't have any invalid xml "
"characters ('>' '&' '<')"))

pattern = str("http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we update the pattern to also match URLs that don't include the HTTP/HTTPS protocols?

Screenshot 2021-02-01 at 12 10 31 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we should. Let me update

@ivermac ivermac force-pushed the 2006-trigger-error-on-url-in-xform-title branch 2 times, most recently from d1eaa24 to 94868a7 Compare February 4, 2021 13:51
@ivermac ivermac force-pushed the 2006-trigger-error-on-url-in-xform-title branch from 94868a7 to 14b0c2c Compare February 4, 2021 13:51
@ivermac ivermac added this to the Week 4 - 5 (2021) milestone Feb 5, 2021
Copy link
Contributor

@DavisRayM DavisRayM left a comment

Choose a reason for hiding this comment

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

LGTM

@DavisRayM DavisRayM merged commit 46fe563 into master Feb 5, 2021
@DavisRayM DavisRayM deleted the 2006-trigger-error-on-url-in-xform-title branch February 5, 2021 12:25
@DavisRayM DavisRayM mentioned this pull request Feb 23, 2021
1 task
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.

Trigger error when xform title matches a url
3 participants