Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
  • Sign in / Register
Minds Frontend
Minds Frontend
  • Project overview
  • Repository
  • Issues 344
  • Merge Requests 57
  • CI / CD
  • Security & Compliance
  • Packages
  • Wiki
  • Snippets
  • Members
  • Collapse sidebar
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • Minds
  • Minds FrontendMinds Frontend
  • Merge Requests
  • !702

Open
Opened 3 weeks ago by Ben Hayward@benhayward.ben
Report abuse

Fixed pattern matching for external and internal page links #2133

  • Overview 6
  • Commits 12
  • Pipelines 9
  • Changes 16
1/1 thread resolved

Summary

Pattern matching for the /p/ pages is broken on production. To replicate, go and click the iOS link in the footer.

Our system is seeing the URL itunes.apple.com/us/app/minds-com/id961771928%3Fmt Seeing that it contains '/p' and thinking that it is an internal page

This happened because the system was looking simply for indexOf('p/')

For this fix I have upgraded that check to a regex check, in a commonly held service.

image

Steps to test

Ignore the first iPhone link - that is invalid data that for now is trapped in the sandbox. (If there is now only 1 link for iPhone, include it in the test, we may have found the missing schema and fixed).

image

  1. Not logged in, check the links on the bottom footer, ensure they're all working
  2. Log in, check the links on the newsfeed footer
  3. Check the links on the actual /p/ pages, like https://www.minds.com/p/terms-of-service/
  4. Check the pro footer hasn't been effected https://www.minds.com/pro

Estimated Regression Scope

The components touched by this are: Footer, register homepage and newsfeed.

I think the worst-case scenario would be a failure in the Regex logic causing another pattern matching issue.

Edited 3 weeks ago by Ben Hayward
Request to merge fix/external-pages-regex-2133 into master
Open in Web IDE
Pipeline #108421721 running for 7d5b8f87 on fix/external-pages-regex-2133
              Requires 3 more approvals from Devs, Deployers, and QA.
              Rami Albatal
              Rami Albatal
              Juan Manuel Solaro
              Juan Manuel Solaro
              Emiliano Balbuena
              Emiliano Balbuena
              Martin Santangelo
              Martin Santangelo
              Brian Hatchet
              Brian Hatchet
              Ready to be merged automatically. Ask someone with write access to this repository to merge this request

              Mentions #2133

              Deletes source branch

              • Ben Hayward @benhayward.ben added Squad::Yellow Status::Requires Changes scoped labels 3 weeks ago

                added scoped labels

              • Ben Hayward @benhayward.ben added 1 commit 3 weeks ago

                added 1 commit

                • 6527b1eb - Update footer.component.spec.ts

                Compare with previous version

              • Ben Hayward @benhayward.ben added 2 commits 3 weeks ago

                added 2 commits

                • 4abf88a3 - Fixed regex issue apparent on sanbox
                • 4a17a6f9 - Merge branch 'fix/external-pages-regex-2133' of gitlab.com:minds/front into...

                Compare with previous version

              • Ben Hayward
                Ben Hayward @benhayward.ben · 3 weeks ago
                Developer

                Still an issue with that regex.

                !702 (diffs)

                Seems to be disregarding the starting char ^

              • Ben Hayward @benhayward.ben added 1 commit 3 weeks ago

                added 1 commit

                • a06f22a5 - Turned regex.test into path.match(regex) because of weird error

                Compare with previous version

              • Ben Hayward @benhayward.ben added 1 commit 3 weeks ago

                added 1 commit

                • bbd796c7 - Fixed RexExp issue

                Compare with previous version

              • Ben Hayward @benhayward.ben changed the description 3 weeks ago

                changed the description

              • Ben Hayward @benhayward.ben added Status::BuddyReview scoped label and automatically removed Status::Requires Changes label 3 weeks ago

                added scoped label and automatically removed label

              • Emiliano Balbuena
                Emiliano Balbuena @edgebal started a thread on an old version of the diff 2 weeks ago
                Resolved by Ben Hayward 6 days ago
              • Emiliano Balbuena @edgebal approved this merge request 2 weeks ago

                approved this merge request

              • Emiliano Balbuena
                Emiliano Balbuena @edgebal · 2 weeks ago
                Developer

                Looks good! Internal links seem to be working OK on the sandbox site.

              • Ben Hayward @benhayward.ben added Status::Requires Changes scoped label and automatically removed Status::BuddyReview label 1 week ago

                added scoped label and automatically removed label

              • Ben Hayward @benhayward.ben added 1 commit 1 week ago

                added 1 commit

                • 267e8228 - Simplified RegExp

                Compare with previous version

              • Ben Hayward @benhayward.ben added Status::BuddyReview scoped label and automatically removed Status::Requires Changes label 6 days ago

                added scoped label and automatically removed label

              • Ben Hayward @benhayward.ben resolved all threads 6 days ago

                resolved all threads

              • Ben Hayward @benhayward.ben assigned to @brianhatchet and @xander-miller 3 days ago

                assigned to @brianhatchet and @xander-miller

              • Mark Harding @markeharding approved this merge request 3 days ago

                approved this merge request

              • Mark Harding @markeharding added Status::Ready to Merge scoped label and automatically removed Status::BuddyReview label 3 days ago

                added scoped label and automatically removed label

              • Emiliano Balbuena
                Emiliano Balbuena @edgebal · 3 days ago
                Developer

                Re-tested, working OK on review site.

              • Emiliano Balbuena @edgebal approved this merge request 3 days ago

                approved this merge request

              • Ben Hayward @benhayward.ben added 15 commits 2 days ago

                added 15 commits

                • 267e8228...7b93f3f1 - 14 commits from branch master
                • 0eb70a77 - Merge branch 'master' into 'fix/external-pages-regex-2133'

                Compare with previous version

              • Ben Hayward
                Ben Hayward @benhayward.ben · 2 days ago
                Developer

                Note, merge conflicts lost this card its approvals from mark and Emi.

                Change was just that both were trying to add an import. Used 'both' merge strat.

              • Xander Miller @xander-miller approved this merge request 19 hours ago

                approved this merge request

              • Ben Hayward @benhayward.ben added Status::Requires Changes scoped label and automatically removed Status::Ready to Merge label 1 hour ago

                added scoped label and automatically removed label

              • Ben Hayward
                Ben Hayward @benhayward.ben · 1 hour ago
                Developer

                Merge conflicts again

              • Ben Hayward @benhayward.ben added 10 commits 35 minutes ago

                added 10 commits

                • 0eb70a77...1c8ee565 - 8 commits from branch master
                • 9260794e - Merge branch 'master' of gitlab.com:minds/front into fix/external-pages-regex-2133
                • 4bfb935d - Merge branch 'fix/external-pages-regex-2133' of gitlab.com:minds/front into...

                Compare with previous version

              • Ben Hayward @benhayward.ben added 1 commit 33 minutes ago

                added 1 commit

                • 7d5b8f87 - Update common.module.ts

                Compare with previous version

              Please register or sign in to reply
              2 Assignees
              Brian Hatchet's avatar
              Xander Miller's avatar
              None
              Milestone
              None
              Time tracking
              No estimate or time spent
              2
              Labels
              Squad::Yellow Status::Requires Changes
              Lock merge request
              Unlocked
              11
              11 participants
              user avatar
              Rami Albatal
              user avatar
              Juan Manuel Solaro
              user avatar
              Emiliano Balbuena
              user avatar
              Martin Santangelo
              user avatar
              Brian Hatchet
              user avatar
              Marcelo Rivera
              user avatar
              Xander Miller
              Reference: minds/front!702

              Now you can access the merge request navigation tabs at the top, where they’re easier to find.

              More information and share feedback