Allow glitch id starting with ~ #10544

irmela · 2020-10-03T17:51:15Z

What type of PR is this? (check all applicable)

Bug Fix

Description

~~This PR improved the regex that is used to validate the glitch id to allow ids starting with ~.~~
This PR improves the parse_id method to remove any prefixed ~ before passing it to the template.

Related Tickets & Documents

resolves #10427

QA Instructions, Screenshots, Recordings

Create a new blog post, add {% glitch ~the-super-tiny-compiler %} and save -> no validation error should appear.

Added tests?

yes
no, because they aren't needed
no, because I need help

Added to documentation?

docs.forem.com
readme
no documentation needed

CLAassistant · 2020-10-03T17:51:21Z

All committers have signed the CLA.

rhymes

The code looks great @irmela! I was wondering though if you were able to actually make it work.

I noticed the following while testing it manually:

The markdown is:

test glitch

{% glitch earthy-course %}

{% glitch ~remote-hands %}

So the first "glitch" has no tilde, the second does. The second never fully loads. I wonder if there's anything we need to change in the template to make those work or if it's just a problem on my end.

irmela · 2020-10-04T10:16:30Z

Good catch @rhymes. I was only looking for some glitch content to appear 🙈

The tilde should not appear in the src tag of the iFrame. Will fix it.

irmela · 2020-10-04T11:10:45Z

I updated the PR description:

~~This PR improved the regex that is used to validate the glitch id to allow ids starting with ~.~~
This PR improves the parse_id method to remove any prefixed ~ before passing it to the template.

rhymes

Thanks for the changes @irmela! I noticed something with the regexp now that you're just going remove the leading tilde from the ID

Great work BTW :)

rhymes · 2020-10-04T13:26:05Z

app/liquid_tags/glitch_tag.rb

@@ -3,6 +3,7 @@ class GlitchTag < LiquidTagBase

  PARTIAL = "liquids/glitch".freeze
  ID_REGEXP = /\A[a-zA-Z0-9\-]{1,110}\z/.freeze
+  TILDE_PREFIX_REGEXP = /\A~*/.freeze


to be 100% correct, this matches "nothing at the beginning of a string", so basically anything you pass. That's because you use the *. As you then replace the ID with nothing is not a big deal but in theory this should simply be:

\A~ which means "begins with tilde"

rhymes · 2020-10-04T13:28:29Z

app/liquid_tags/glitch_tag.rb

@@ -36,6 +37,7 @@ def valid_id?(input)

  def parse_id(input)
    id = input.split(" ").first
+    id.gsub!(TILDE_PREFIX_REGEXP, "")


no need to gsub! if the regexp uses \A since it's just going to scan the beginning of the string, sub! is fine:

[2] pry(main)> "~some-id".gsub!(/\A~/, "") => "some-id" [3] pry(main)> "~some-id".sub!(/\A~/, "") => "some-id"

Use sub instead of gsub as we're only interested in removing the first occurence of ~

rhymes

LGTM! :)

maestromac

Thank you for this PR @irmela . LGTM!

* chore: update the profile to be more dynamic. Maps out the group + field name + value for field * feat: add the correct input type to the form tying it to the correct method to update * feat: update the value and the label * feat: update the profile with a submit * refactor: ensure we dont show empty groups on the settings/profile * fix: this shoudl be dribbble * fix: use attributes! to refresh the cache to ensure that we font run into the case where Profile.attributes is [] * feat: show group description only when it exists * chore: add some classes * feat: use the profile update service to update the relevant user information * chore: rename variable * chore: use a scope to not return empty groups * fix: only update user if the profile sync is update is successful. Also capture errors * feat: style the crayons field checkbox * refactor: rename the attribute to user instead of profile * refactor: service object * Fix CodeClimate warning and rearrange code * Fix service object, add missing attribute * Fix flash and add redirect * Fix service object + spec * Fix user edits profile spec * Disable spec * Fix more specs * Undo vim fail * Add data update scripts * Update where clause in data update script * Rename method * Make sync_to_user conditional * Add after update actions and Honeycomb * feat: handle checkbox allow chcekbox to check * Don't set experience cookie in ProfilesController * Clean up Profiles::Update object * Fix spec * Remove Twitch integration (#10732) * Remove Twitch integration * Ignore columns * Change admin password in docs (#10735) Update default admin password in documentation * Do not require meta_keywords to be set (#10721) * Remove extra character from meta_keywords in /listings/index.html.erb * Remove meta_keywords from MANDATORY_CONFIGS * Add and use meta_keywords_tag helper * Use modern tag syntax instead of deprecated syntax * Add and use meta_keywords_default helper * Add and use meta_keywords_article helper * Remove * from meta_keywords_field.label * Update meta_keyword specs to account for no keywords being set * Generalizes Connect welcome message and settings nav tab and refactors tests (#10741) [deploy] * [deploy] fix: Clear chat input after Enter key submit (#10487) * fix: Clear chat input after Enter key submit * fix: Rename Composer component * test: Add integration tests for input value * After pressing Enter * After clicking Send * After clicking Save edit * After clicking Close edit * Remove tabindex of -1 on upload image input in the write a post editor (#10543) * Remove tabindex of -1 on upload image input in the write a post editor * Remove negative tabindex on image uploader's outer button * Give tabindex of -1 to outer button on image uploader * Update buttons.scss * [deploy] Add recaptcha keys to as site-configurable keys (#10736) * Add recaptcha keys to as site-configurable keys * Add recaptcha to site config view * Use site config key * Make ReCAPTCHA tag optional if keys are blank * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Refactor logic for readability * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Use proper capitalization of recaptcha Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> * Add Section for Series/Collections (#10719) * Add Section for Series/Collections I added a small section in this area of the docs to share some information about collections/ series. There is not much information in the documentation about collections or series and I felt like this would be beneficial to others looking to help with the Series feature of the website. * Updated wording of collections section. * Articles API: filtering by tags (#3654) (#10614) * Articles API: filtering by tags (#3654) * Address CR comments (#3654) * Add test case * Move tests * Update docs * Use _any_ option for querying by tags (#3654) * Move Imgproxy endpoint config to Images::Optimizer (#10742) * Link Email Account creation UI to existing Create Account UI (#10746) [deploy] * Conditional button to link to email accout creation * Rename email signup state param * Test for email account creation button scenarios * Add forem meta tags (#10747) * Add forem meta tags * Add docs * Update docs/technical-overview/compatibility.md Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com> * Update docs/technical-overview/compatibility.md Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com> * Update docs/technical-overview/compatibility.md Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com> * Update docs/technical-overview/compatibility.md Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com> * Remove Twitch-related columns from users table (#10750) * [deploy] Long text goes outside cards (#10740) * Add word-break to crayons-card Add word-break to crayons-card this will allow words to be broken if the word is too long but will wrap normally otherwise. * Use overflow-wrap: anywhere; `break-word` is deprecated though `anywhere` does not work in IE * Removes superfluous sign_in from users_onboarding_spec.rb (#10757) [deploy] * [deploy] Allow glitch id starting with ~ (#10544) * [deploy] crayons prep (#10737) * Add DEV special case for following hiring tag * Move image validation to Profiles::Update object * spec: move over the tests for validating images * Move follow_hiring_tag into the service object Co-authored-by: Ridhwana <ridhwana.khan16@gmail.com> Co-authored-by: Andrew Bone <AndrewB05@gmail.com> Co-authored-by: Vaidehi Joshi <vaidehi.sj@gmail.com> Co-authored-by: Julianna Tetreault <32834804+juliannatetreault@users.noreply.github.com> Co-authored-by: Robin Gagnon <me@reobin.dev> Co-authored-by: Manaswini <58681405+Manaswini1832@users.noreply.github.com> Co-authored-by: Andy Zhao <17884966+Zhao-Andy@users.noreply.github.com> Co-authored-by: Christopher Wray <53663762+cwray-tech@users.noreply.github.com> Co-authored-by: Rafal Trojanowski <rt.trojanowski@gmail.com> Co-authored-by: Mac Siri <mac@dev.to> Co-authored-by: Josh Puetz <josh@dev.to> Co-authored-by: Ben Halpern <bendhalpern@gmail.com> Co-authored-by: Jacob Herrington <jacobherringtondeveloper@gmail.com> Co-authored-by: irmela <irmela@berlin-coding.de> Co-authored-by: ludwiczakpawel <ludwiczakpawel@gmail.com>

Allow glitch id starting with ~

a02a0d0

pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Oct 3, 2020

rhymes reviewed Oct 4, 2020

View reviewed changes

irmela added 2 commits October 4, 2020 12:42

Remove tilde prefix when parsing id

7ad322b

Use double quotes

ff2ce23

rhymes reviewed Oct 4, 2020

View reviewed changes

Improve regex

92aba26

Use sub instead of gsub as we're only interested in removing the first occurence of ~

rhymes approved these changes Oct 4, 2020

View reviewed changes

pr-triage bot added PR: partially-approved bot applied label for PR's where a single reviewer approves changes and removed PR: unreviewed bot applied label for PR's with no review labels Oct 4, 2020

rhymes requested review from maestromac and Zhao-Andy October 4, 2020 14:14

maestromac approved these changes Oct 9, 2020

View reviewed changes

maestromac merged commit ee989a0 into forem:master Oct 9, 2020

pr-triage bot added PR: merged bot applied label for PR's that are merged and removed PR: partially-approved bot applied label for PR's where a single reviewer approves changes labels Oct 9, 2020

citizen428 pushed a commit that referenced this pull request Oct 12, 2020

[deploy] Allow glitch id starting with ~ (#10544)

259d246

boardfish pushed a commit to boardfish/forem that referenced this pull request Oct 17, 2020

[deploy] Allow glitch id starting with ~ (forem#10544)

cf78b23

Allow glitch id starting with ~ #10544

Allow glitch id starting with ~ #10544

irmela commented Oct 3, 2020 •

edited by maestromac

CLAassistant commented Oct 3, 2020 •

edited

rhymes left a comment •

edited

irmela commented Oct 4, 2020 •

edited

irmela commented Oct 4, 2020

rhymes left a comment

rhymes Oct 4, 2020

rhymes Oct 4, 2020

rhymes left a comment

maestromac left a comment

Allow glitch id starting with ~ #10544

Allow glitch id starting with ~ #10544

Conversation

irmela commented Oct 3, 2020 • edited by maestromac

What type of PR is this? (check all applicable)

Description

Related Tickets & Documents

QA Instructions, Screenshots, Recordings

Added tests?

Added to documentation?

CLAassistant commented Oct 3, 2020 • edited

rhymes left a comment • edited

Choose a reason for hiding this comment

irmela commented Oct 4, 2020 • edited

irmela commented Oct 4, 2020

rhymes left a comment

Choose a reason for hiding this comment

rhymes Oct 4, 2020

Choose a reason for hiding this comment

rhymes Oct 4, 2020

Choose a reason for hiding this comment

rhymes left a comment

Choose a reason for hiding this comment

maestromac left a comment

Choose a reason for hiding this comment

irmela commented Oct 3, 2020 •

edited by maestromac

CLAassistant commented Oct 3, 2020 •

edited

rhymes left a comment •

edited

irmela commented Oct 4, 2020 •

edited