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

Fix map errors with new LEL 2.2.1 #7545

Merged
merged 7 commits into from
Feb 22, 2020
Merged

Conversation

nstjean
Copy link
Contributor

@nstjean nstjean commented Feb 19, 2020

Fixes #7381 (<=== Add issue number here)

  • Fixes the errors that were still happening after the new LEL version
  • Using the display param in LEL
  • Changes the peopleLeaflet inline map over to using the new PLpeople layer in LEL
  • Cleaned up /people, it also uses the peopleLeaflet template
  • Fixes a bug where some new code from fix FIX subscription share popover not disappearing #7386 was interfering with maps
  • Now showing people (using PLpeople layer) and site content on /map
  • Fixed a bug where layer marker popups weren't appearing when clicked on inline maps, but were appearing on /people and /map
  • Used webmock to stub out the api used for PLpeople layer - it is currently very slow and was causing test failures

/map showing popup for a content node
FireShot Capture 302 - 🎈 Public Lab_ Map - localhost

/map showing popup for a person
FireShot Capture 301 - 🎈 Public Lab_ Map - localhost

/wiki/inline-maps
FireShot Capture 300 - 🎈 Public Lab_ Inline Maps - localhost

/people
FireShot Capture 299 - 🎈 Public Lab_ List - localhost

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

@nstjean nstjean requested review from a team as code owners February 19, 2020 20:59
@codecov
Copy link

codecov bot commented Feb 19, 2020

Codecov Report

Merging #7545 into master will increase coverage by 0.15%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7545      +/-   ##
==========================================
+ Coverage   81.87%   82.03%   +0.15%     
==========================================
  Files          97       97              
  Lines        5612     5615       +3     
==========================================
+ Hits         4595     4606      +11     
+ Misses       1017     1009       -8
Impacted Files Coverage Δ
app/controllers/users_controller.rb 82.29% <100%> (+0.18%) ⬆️
app/channels/application_cable/connection.rb 100% <0%> (ø) ⬆️
app/channels/application_cable/channel.rb 100% <0%> (ø) ⬆️
app/channels/user_notification_channel.rb 83.33% <0%> (ø) ⬆️
app/channels/user_channel.rb 83.33% <0%> (ø) ⬆️
app/channels/room_channel.rb 71.42% <0%> (ø) ⬆️
app/controllers/openid_controller.rb 53.84% <0%> (ø) ⬆️
app/controllers/comment_controller.rb 87.23% <0%> (ø) ⬆️
app/controllers/subscription_controller.rb 69% <0%> (ø) ⬆️
... and 18 more

@nstjean nstjean closed this Feb 19, 2020
@nstjean nstjean reopened this Feb 19, 2020
@nstjean
Copy link
Contributor Author

nstjean commented Feb 21, 2020

The test/system/post_test.rb passes on my local. It's throwing an error in travis. Have to figure out why!

FireShot Capture 303 - Job #16776 5 - publiclab_plots2 - Travis CI - travis-ci org

@nstjean nstjean changed the title Fix map errors with new LEL 2.2.1 WIP: Fix map errors with new LEL 2.2.1 Feb 21, 2020
@nstjean
Copy link
Contributor Author

nstjean commented Feb 21, 2020

Ok so commenting out the PLpeople layer made the tests pass. I suspect it's because that API takes so long to load. I'm going to try to stub it out for the test.

@nstjean nstjean force-pushed the map-error-lel branch 4 times, most recently from afc799f to fc43734 Compare February 21, 2020 19:17
@plotsbot
Copy link
Collaborator

1 Warning
⚠️ There was an error with Danger bot’s Junit parsing: No JUnit file was found at output.xml
3 Messages
📖 @nstjean Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.
📖 It looks like you haven’t marked all the checkboxes. Help us review and accept your suggested changes by going through the steps one by one. If it is still a ‘Work in progress’, please include ‘[WIP]’ in the title.
📖 #
Screenshots 📸 (click to expand)

7545-test_questions.png

7545-test_embeddable_grids.png

7545-test_signup.png

7545-test_viewing_the_settings_page.png

7545-test_tag_by_author_page.png

7545-test_wiki_page_with_inline_grids.png

7545-test_stats.png

7545-test_viewing_the_dashboard.png

7545-test_searching_an_item_from_the_homepage.png

7545-test_signup_modal_form_validation.png

7545-test_tag_stats.png

7545-test_login_modal_form_validation.png

7545-test_questions_shadow.png

7545-test_login_modal.png

7545-test_profile_page.png

7545-test_comments.png

7545-test_tags.png

7545-test_signup_modal.png

7545-test_wiki.png

7545-test_methods.png

7545-test_tag_page.png

7545-test_blog_page_with_location_modal.png

7545-test_tag_wildcard.png

7545-test_signup_modal_disabled_submit_button_on_empty_username.png

7545-test_embeddable_thumbnail_grids.png

7545-test_front_page_with_navbar_search_autocomplete.png

7545-test_spam_moderation_page.png

7545-test_login.png

7545-test_viewing_the_dropdown_menu.png

7545-test_viewing_question_post.png

7545-test_mobile_displays.png

7545-test_simple-data-grapher_powertag.png

7545-test_front.png

7545-test_question_page.png

7545-test_tag_contributors_page.png

7545-test_blog.png

7545-test_people.png

7545-test_wiki_revisions.png

Learn about automated screenshots

Generated by 🚫 Danger

body: "{items:[]}",
status: 200,
:headers => {"Content-Type"=> "application/json"}
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this worked!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can mock any testing framework now 😄 🎉 !!!

@nstjean nstjean changed the title WIP: Fix map errors with new LEL 2.2.1 Fix map errors with new LEL 2.2.1 Feb 21, 2020
@nstjean
Copy link
Contributor Author

nstjean commented Feb 21, 2020

Ready for review!

Copy link
Collaborator

@cesswairimu cesswairimu left a comment

Choose a reason for hiding this comment

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

Wow the maps look great 🎉 🎉 Superb 🚀

@cesswairimu cesswairimu merged commit d4018a2 into publiclab:master Feb 22, 2020
@cesswairimu
Copy link
Collaborator

@sagarpreet-chadha checkout these.. Really cool


var setHash = 1;
<% if defined? url_hash != nil and url_hash == 0 %>
setHash = 0;
<% end %>

var options<%= unique_id %> = {
mainContent: "people",
setHash: setHash
layers: ["PLpeople"]
Copy link
Contributor

Choose a reason for hiding this comment

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

YESSS

body: "{items:[]}",
status: 200,
:headers => {"Content-Type"=> "application/json"}
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can mock any testing framework now 😄 🎉 !!!

@sagarpreet-chadha
Copy link
Contributor

I am so happy to see PLpeople to go LIVE!!! This layer is one of the best layer i had the opportunity to work on. I think i can take a session on how i implemented this heatmap if you guys are interested 😄 .
Enough of self promotion :P

Thanks @nstjean , @cesswairimu for completing this 🎉

@jywarren
Copy link
Member

🎉 Broke out a couple follow-ups here!

publiclab/leaflet-blurred-location-display#98 and a little simpler still: https://github.com/publiclab/leaflet-blurred-location-display/issues/97

icarito pushed a commit that referenced this pull request Mar 31, 2020
* fix inline map errors with using new LEL 2.2.1, add plpeople to /map and /people

* move subscription popover javascript fix to subscription page to prevent it interfering with maps

* calculate lat, lon, zoom for people map using global or local variable

* fix bug where map popup doesn't show on inline maps

* fix codeclimate error

* troubleshooting travis error

* add webmock to stub api requests
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.

Sidebar map layers not working
5 participants