Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
  • Sign in / Register
Minds Frontend
Minds Frontend
  • Project overview
  • Repository
  • Issues 350
  • Merge Requests 56
  • 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 2 weeks ago by Ben Hayward@benhayward.ben
Report abuse

Fixed pattern matching for external and internal page links #2133

  • Overview 4
  • Commits 8
  • Pipelines 6
  • 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 2 weeks ago by Ben Hayward
Request to merge fix/external-pages-regex-2133 into master
The source branch is 14 commits behind the target branch
Open in Web IDE
Pipeline #106855149 passed with warnings for 267e8228 on fix/external-pages-regex-2133
              Merge request approved. Approved by
              Emiliano Balbuena
              Emiliano Balbuena
              Mark Harding
              Mark Harding
              Guy Thouret
              Guy Thouret
              Olivia Madrid
              Olivia Madrid
              Xander Miller
              Xander Miller
              Juan Manuel Solaro
              Juan Manuel Solaro
              Marcelo Rivera
              Marcelo Rivera
              There are merge conflicts. Resolve these conflicts or ask someone with write access to this repository to merge it locally

              Mentions #2133

              Deletes source branch

              You can merge this merge request manually using the
              • Ben Hayward @benhayward.ben added Squad::Yellow Status::Requires Changes scoped labels 2 weeks ago

                added scoped labels

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

                added 1 commit

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

                Compare with previous version

              • Ben Hayward @benhayward.ben added 2 commits 2 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 · 2 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 2 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 2 weeks ago

                added 1 commit

                • bbd796c7 - Fixed RexExp issue

                Compare with previous version

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

                changed the description

              • Ben Hayward @benhayward.ben added Status::BuddyReview scoped label and automatically removed Status::Requires Changes label 2 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 3 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 5 days ago

                added scoped label and automatically removed label

              • Ben Hayward @benhayward.ben added 1 commit 4 days 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 4 days ago

                added scoped label and automatically removed label

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

                resolved all threads

              • Ben Hayward @benhayward.ben assigned to @brianhatchet and @xander-miller 17 hours ago

                assigned to @brianhatchet and @xander-miller

              • Mark Harding @markeharding approved this merge request 4 hours ago

                approved this merge request

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

                added scoped label and automatically removed label

              • Emiliano Balbuena
                Emiliano Balbuena @edgebal · 1 hour ago
                Developer

                Re-tested, working OK on review site.

              • Emiliano Balbuena @edgebal approved this merge request 1 hour ago

                approved this merge request

              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::Ready to Merge
              Lock merge request
              Unlocked
              11
              11 participants
              user avatar
              Guy Thouret
              user avatar
              Olivia Madrid
              user avatar
              Xander Miller
              user avatar
              Juan Manuel Solaro
              user avatar
              Marcelo Rivera
              user avatar
              Brian Hatchet
              user avatar
              Martin Santangelo
              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