Fixed pattern matching for external and internal page links #2133
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.
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).
- Not logged in, check the links on the bottom footer, ensure they're all working
- Log in, check the links on the newsfeed footer
- Check the links on the actual /p/ pages, like https://www.minds.com/p/terms-of-service/
- 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.
added scoped labels
- Developer
added 1 commit
- a06f22a5 - Turned regex.test into path.match(regex) because of weird error
changed the description
added scoped label and automatically removed label
1 import { Injectable } from '@angular/core'; 2 3 /** 4 * Shared logic between internal 'p/' page loading components. 5 * e.g. header, footer. 6 * 7 * @author Ben Hayward 8 */ 9 @Injectable() 10 export class PagesService { 11 private internalPageRegex: RegExp = /^p\/.+$/gm; // matches 'p/' in first and second position. - Developer
Since you're just using this regular expression for testing, it can be simplified to:
/^\/?p\/./
That will match:
- Start of the line
- An optional slash
- The letter p
- A slash
- A character that's not a line terminator
g
andm
modifiers are unnecessary since you're not needing the matches information neither testing a multi-line string.Edited by Emiliano Balbuena
approved this merge request
- Developer
Looks good! Internal links seem to be working OK on the sandbox site.