Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid 404 error when clicking on country database #44

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

licataae
Copy link

When someone would click on countries in the interactive world map on the homepage, a 404 error would occur - I added an explicit path for the profiles list & used re.escape instead of compile to escape special characters in search terms for regex matching. I tested the changes on a local deployment using 100, 500 and 1000 profiles and it worked just fine, but let me know if I can test anything else out to be sure. Thanks!

error_database

@anibalsolon anibalsolon self-requested a review June 28, 2024 20:31
Copy link
Member

@anibalsolon anibalsolon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @licataae thank you for the PR!

The escape is a great idea, it should work much better.

I believe we could take another approach: change the map interaction to open directly the correct URL: /repo/?s=Canada . WYT? We may have other places that it occurs:

https://github.com/WomenInNeuroscience/winrepo/blob/main/profiles/templates/profiles/home.html
https://github.com/WomenInNeuroscience/winrepo/blob/main/profiles/static/profiles/js/home-graphs.js#L99

@licataae
Copy link
Author

Hi @anibalsolon, thank you for this information & for your reply! I'm not sure I understand your suggested approach / the file where we should alter the map interaction. Do you suggest modifying the DrawMap function in home-graphs.js to directly call on the correct URL? e.g., line 99 of home-graphs.js: window.location.href = '/list?s=' + encodeURIComponent(country) ? Thank you very much!

@anibalsolon
Copy link
Member

@licataae
Copy link
Author

licataae commented Jul 4, 2024

Ah, got it! I just updated these two files & tested it out, now the map & profiles load the URL directly... I force pushed the new commit instead of the old (let me know if there's a smoother way to go about this). Thanks!

@licataae licataae closed this Jul 4, 2024
@licataae licataae reopened this Jul 4, 2024
@anibalsolon anibalsolon self-requested a review July 16, 2024 13:04
Copy link
Member

@anibalsolon anibalsolon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's perfect, @licataae ! Much appreciated 🙏🏻

@anibalsolon anibalsolon changed the base branch from main to develop July 16, 2024 13:05
@anibalsolon anibalsolon changed the base branch from develop to main July 16, 2024 13:05
@anibalsolon anibalsolon merged commit 20f646f into WomenInNeuroscience:main Jul 16, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants