- Issue created by @Anybody
- Assigned to nidhi27
- Issue was unassigned.
- Status changed to Needs review
8 months ago 2:09pm 5 April 2024 - 🇮🇳India nidhi27
Hi,
I have created #MR14 with the path validations. I have checked the functionality. Its working as expected. Let me know if anything else needs to be add here.
Thanks!
- 🇩🇪Germany Anybody Porta Westfalica
@nidhi27 looking great!
I left some comments. What about edge-cases? Is there anything we need to think about for URLs, like external URLs or anchor links (#)? Does that make sense for such cases?
External links don't make sense, I think? And Anchors?
Anything else?Could you also add tests for the new functionality perhaps? And if it's possible and you have the time and coding fun, perhaps general tests in 📌 Write tests Active ?
- 🇮🇳India nidhi27
Hi @Anybody,
Other than this, links can have query parameters. This is also getting verified here with anchor links(#).
I dont think external links is making sense here. Mostly front pages are inner pages and thats why we are checking it with "/" and if the url exist in drupal or not.I think we can add all the test cases in https://www.drupal.org/project/front/issues/3438834 📌 Write tests Active to avoid any code conflicts.
- Assigned to Grevil
- 🇩🇪Germany Anybody Porta Westfalica
Yes I agree, please prepare the tests and I'll pass this for review to Mr. clean code @Grevil :)