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

DM 3631: Support /communities URL for Community pages #551

Merged
merged 11 commits into from
Oct 19, 2022

Conversation

camillevilla
Copy link
Collaborator

@camillevilla camillevilla commented Oct 14, 2022

JIRA issue link

DM-3631

Description - what does this code do?

  • updates the webdrivers gem to fix an issue affecting local dev for M1 Macs (Webdrivers::NetworkError, probably due to new chromedriver naming titusfortner/webdrivers#237)
  • Adds support for /communities/:page_group/:page_slug/ in the
  • Removes unused subpage breadcrumb code that was creating an extra breadcrumb for this use case
    extra-breadcrumb
  • adds a COMMUNITIES constant to navigation helper that includes xr-network
  • Edit breadcrumbs so that a community subpage always routes back to community homepage (e.g. /communities/xr-network/events has a breadcrumb back to /communities/xr-network)
  • Adds a redirect for /communities

Testing done - how did you test it/steps on how can another person can test it

  1. Go to /admin/page_groups and create two page groups: xr-network and other
  2. Go to /admin/pages and create two subpages: /xr-network/test and /other/test
  3. Go to each page. Neither page should have breadcrumbs.
  4. Go to /communities/xr-network/test. The content should be the same as /xr-network/test.
  5. Go to admin/pages and create two home pages: /xr-network/home and /other/home.
  6. Go to /communities/xr-network/test. The breadcrumb should link to `/communities/xr-network.
  7. Go to /other/test. The breadcrumb should link to /other.
  8. Edit /xr-network/test. Add an image component. Add a downloadable file component. Add a practice card list component.
  9. Go to /xr-network/test and click on all page component links to make sure they still work as expected.
  10. Go to /communities. The page should redirect to /communities/xr-network.

Screenshots, Gifs, Videos from application (if applicable)

Link to mock-ups/mock ups (image file if you have it) (if applicable)

Acceptance criteria

  • Determine how URLs are used for other pages
  • Determine how many things we can go deep
  • Can we do “communities/xr-network/PageName”
  • QA URLs
  • QA images
  • QA jumplinks

Make sure this doesn't break other pages

Definition of done

  • Unit tests written (if applicable)
  • e2e/accessibility tests written (if applicable)
  • Events are logged appropriately
  • Documentation has been updated, if applicable
  • A link has been provided to the originating JIRA issue
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs

@camillevilla camillevilla force-pushed the dm-3631-communities-url branch from 8962a6a to 25757c0 Compare October 17, 2022 17:29
Copy link
Contributor

@bradjohnson92008 bradjohnson92008 left a comment

Choose a reason for hiding this comment

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

Everything worked for me. Great job on this Camille.

Copy link
Contributor

@JoshingYou1 JoshingYou1 left a comment

Choose a reason for hiding this comment

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

Nice work on this Camille! I left a few comments. I'm also wondering if we should add a redirect in the controller, so if the user tries to visit xr-network/events, they get automatically routed to /communities/xr-network/events. With this suggestion, we may want to add a constant to the PageGroup class, kind of like how we did PRACTICE_EDITOR_SLUGS, and just call it COMMUNITY_SLUGS or something. That way we could just call it whenever we need to.

def add_sub_page_breadcrumb
session[:breadcrumbs] << { 'display': "#{@page.title}", 'path': @builder_page_path }
end

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 we should keep this code. It was there for instances when the user navigated from a subpage to another page. For example, If the subpage had practices, when they clicked on the card and landed on the practice's show page, they would have a breadcrumb for both the landing page and the subpage

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I've updated the branch so it retains that method.

elsif @builder_landing_page.exists?
empty_breadcrumbs
add_landing_page_breadcrumb("/#{params[:page_group_friendly_id]}")
add_sub_page_breadcrumb
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding to the comment above, we could probably add some code like

is_communities_page = COMMUNITIES.include?(@page.page_group.slug)

somewhere above the if/else, and then do:

add_sub_page_breadcrumb unless COMMUNITIES.include?(@page.page_group.slug)

# Accept pages with /communities prefix, e.g. /communities/xr-network/news
get '/communities/:page_group_friendly_id' => "page#show"
get '/communities/:page_group_friendly_id/:page_slug' => 'page#show'
get '/communities', to: redirect('/communities/xr-network') # temporary redirect until more communities are added
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 this is a good work-around! I'm wondering if we should think of a way to make the double-nested routes dynamic for future cases, so we don't have to continually add new routes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@JoshingYou1 do you mean Line 133? This route should get removed once we have a second community (and ideally have a Communities home page).

@camillevilla camillevilla force-pushed the dm-3631-communities-url branch from 25757c0 to 0e7d9e2 Compare October 17, 2022 22:44
@camillevilla
Copy link
Collaborator Author

Ok, so far I have

  1. removed the commit that touched the subpage breadcrumbs.
  2. moved community slugs to a method in the PageGroup model.

I'm still researching redirecting users away from xr-network/events to /communities/xr-network/events.

Copy link
Contributor

@JoshingYou1 JoshingYou1 left a comment

Choose a reason for hiding this comment

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

Nice refactor Camille! It redirects like a charm now 😄 For V2, we should definitely add a test to cover this scenario.

@camillevilla camillevilla force-pushed the dm-3631-communities-url branch from 3c23e32 to f371d66 Compare October 18, 2022 21:07
@camillevilla camillevilla force-pushed the dm-3631-communities-url branch from cd38b6d to 6d778ea Compare October 18, 2022 22:09
@camillevilla camillevilla merged commit 368d3ac into feature/communities Oct 19, 2022
@camillevilla camillevilla deleted the dm-3631-communities-url branch October 19, 2022 15:47
JoshingYou1 added a commit that referenced this pull request Oct 21, 2022
* Dm 3533 (#543)

* DM-3533: Added new component and component image models

* DM-3533: Started 'Add Image' functionality

* DM-3533: Removed TinyMCE code that was messing with WYSIWYG links

* DM-3533: Fixed TinyMCE issues

* DM-3533: Added more fields and added delete functionality to component images

* DM-3533: Finished all backend functionality

* DM-3533: Added some front end code for compound body component

* DM-3533: Added configurable bottom and top margin. Cleaned up some code

* DM-3533: Added admin panel tests for CompoundBodyComponent

* DM-3533: Added a bunch of tests

* DM-3533: Added unit tests for new models

* DM-3533: Cleaned up some code

* DM-3533: Fixed some failing tests

* DM-3533: Fixed failing tests

* DM-3533: Refactored some code and added another test

* DM-3533: Reverted data type for paperclip attachments

* DM-3533: Added to Pages show page and a few tests

* DM-3533: Added mobile styling and another test

* DM-3533: Cleaned up some code

* DM-3533: Commented out selenium_chrome line from spec_helper

* DM-3533: Added outline to images that belong to a CompoundBodyComponent

* DM-3533: Removed unnecessary 'validate' call

* DM-3533: Added external link styling to WYSIWYG links for compound body component

* DM-3533: Removed some unnecessary code

* DM-3533: Cleaned up some JS code

* DM-3533: Fixed issue with external link styling for PageComponentImages

* DM-3533: Added ':thumb' style to PageComponentImage on Page show page

* Dm 3592 anchor tags (#542)

* dm-3592

add fix for anchor tag/links in chrome.

* dm-3592

build out anchor tags for all Accordions/(FAQs) on a page.

* dm-3592

test to ensure accordion component displays

* dm-3592

spec - add click event to display accordion text.

* dm-3592

spec for accordion

* dm-3592

* dm-3508

moved anchor tag to div container instead of above div container.

* dm-3592

Add test for multiple accordions and anchor tags.

* dm-3508

test

* dm-3508

* dm-3508

PR fixes for anchor tags.

* Dm 3529 (#546)

* DM-3533: Added new component and component image models

* DM-3533: Started 'Add Image' functionality

* DM-3533: Removed TinyMCE code that was messing with WYSIWYG links

* DM-3533: Fixed TinyMCE issues

* DM-3533: Added more fields and added delete functionality to component images

* DM-3533: Finished all backend functionality

* DM-3533: Added some front end code for compound body component

* DM-3533: Added configurable bottom and top margin. Cleaned up some code

* DM-3533: Added admin panel tests for CompoundBodyComponent

* DM-3533: Added a bunch of tests

* DM-3533: Added unit tests for new models

* DM-3533: Cleaned up some code

* DM-3533: Fixed some failing tests

* DM-3533: Fixed failing tests

* DM-3533: Refactored some code and added another test

* DM-3533: Reverted data type for paperclip attachments

* DM-3533: Added to Pages show page and a few tests

* DM-3533: Added mobile styling and another test

* DM-3533: Cleaned up some code

* DM-3533: Commented out selenium_chrome line from spec_helper

* DM-3533: Added outline to images that belong to a CompoundBodyComponent

* DM-3533: Removed unnecessary 'validate' call

* DM-3533: Added external link styling to WYSIWYG links for compound body component

* DM-3533: Removed some unnecessary code

* DM-3533: Cleaned up some JS code

* DM-3529: Added image and image alt text fields to page

* DM-3533: Added ability to add image to a Page in the admin panel

* DM-3533: Fixed issue with external link styling for PageComponentImages

* DM-3533: Added ':thumb' style to PageComponentImage on Page show page

* DM-3529: Working on frontend for header image

* DM-3529: Added mobile styles

* DM-3529: Adjusted styles based on design's feedback

* DM-3529: Finished writing tests and cleaning up styling

* DM-3529: Improved a test

* DM-3529: Fixed failing tests

* DM-3529: Fixed up the schema and uncommented headless chrome

* DM-3529: Removed redundant 'private' call and cleaned up a little bit of code

* DM-3534 DM-3561: Add Events and News Components (#544)

Co-authored-by: Camille Villa <[email protected]>

* Dm 3508 map component (#541)

* dm-3508

Adding support/dependencies for map component.

* dm-3508

Page Builder UI

* dm-3508

* dm-3508

documenting page_map_components schema

* dm-3508

backend and infrastructure for front end.

* dm-3508

* dm-3508

bus logic: fetching diffusion histories for selected practices.

* dm-3508

* dm-3508

center map and zoom

* dm-3508

styles, zoom level

* dm-3508

Fix query for adopting facilities

* dm-3508

removed dbg

* dm-3508

removed dbgs

* dm-3508

marker infowindow

* dm-3508

info window

* dm-3508

logic for statuses

* dm-3508

front end stuff

* dm-3508

info window styling text changes.

* dm-3508

border radius..

* dm-3508

map zoom

* dm-3508

border padding for info window card style

* dm-3508

use padding shorthand

* dm-3508

add margin between description and map bottom.

* dm-3508

info window static size.

* dm-3508

infowindow styles...

* dm-3508

Added in Abbr Short_name for info window content.. i.e., 23 "XR" innovations.. for a specific Facility.

* dm-3508

margin above num innovations in infowindow

* dm-3508

schema change

* dm-3508

added test.

* dm-3508

* dm-3508

fixed issue with text wrapping and ignoring padding.

* dm-3508

info window styles

* dm-3508

make map component mobile friendly.

* dm-3508

more info window and border styles for map component info window.

* dm-3508

* dm-3508

* dm-3508

info window styles

* dm-3508

addressing testing and PR comments.

* dm-3508

refactored some code to use ActiveRecord vs arrays.

* dm-3508

Refactored some code in the controller.

* dm-3508

renamed infowindow partial to _page_map_infowindow

* dm-3508

another slight refactor...

* dm-3508

UPdate based on PR comments.

* dm-3508

Fixed styling for Info Window Facility Link.

* dm-3508

Map and info window styles.. enums for Diffusion History statuses.. etc.

* dm-3508

Addend links for innovations within the map component markers info-window

* dm-3508

forgot to update these changes in schema.rb

* dm-3508

remove commented code and fix margin in info window.

* dm-3508

Added model test.

* dm-3508

more tests for map

* dm-3508

try to fix test.

* dm-3508

Fix test.

* dm-3508

* dm-3508

ok.. lets try different syntax..

* m-3508

display-none?

* dm-3508

forgot the div

* dm-3508

* dm-3508

* dm-3508

* dm-3508

info window styles

* dm-3508

* dm-3508

info window size and positioning

* dm-3508

* dm-3508

* dm-3508

Style changes per design.

* dm-3508

spec for clicking map marker and displaying infowindow

* dm-3508

err in spec.

* dm-3508

Admin Panel test page for creating a map component

* dm-3508

removed debuggers

* dm-3508

Admin panel spec

* dm-3508

Update IDs to match db change for "display_successful_adoptions" for spec.

* Revert "dm-3508"

This reverts commit 8e352bd

* dm-3508

re-factor spec

* dm-3508

more refactor for specs

* dm-3508

* dm-3508

info window spec

* dm-3508

setting ids back to be consistent in map_page_comonent_form.html.arb

* dm-3508

find the map marker...

* dm-3508

Remove other community features from schema that are in other component branches.

* dm-3508

PR updates.

* dm-3508

PR updates

* dm-3508

pr suggestions

* dm-3508

moved helper method to model - refactored method.

* dm-3508

* dm-3508

test not working now...

* dm-3508

PR Comments

* dm-3508

InfoWindow refactors

* dm-3508

Update/Refactor short_name column to map_info_window_text

* dm-3508

updated page_map_component.short_name field to map_info_window_text.

* dm-3508

Update tests for map due to column name change.

* dm-3508

update spec for page_map_component field name change.

* dm-3508

pr comments.

* dm-3508

INfo Window styles.

* dm-3508

Decrease maxWidth and maxHeight

* dm-3508

removed commented code from spec.

* dm-3508

adjustedpixel offset for new maxWidth, maxHeight.

* dm-3508

Fix indent for comment headers.

* dm-3508

revert back to "if" instead of "unless"

* Removed duplicate code in admin/pages.rb

Co-authored-by: Joshua Drumm <[email protected]>

* Community Feature

replace total adoption count with just count of "adopted practices within the community"

* Changed '.count' to '.length' for practice_data array

* DM-3626: Refactored grid column setup for 'CompoundBody' component (#549)

* Dm compound body spacing fix (#550)

* Renamed 'margin' columns to 'padding' and refactored the code to reflect those changes

* Reverted some schema changes

* Fixed a failing test

* Fixed a couple of typos

* DM 3631: Support /communities URL for Community pages (#551)

Co-authored-by: Camille Villa <[email protected]>
Co-authored-by: Joshua Drumm <[email protected]>

* DM-3627: Adjusted styles to align with DM homepage styles (#553)

* Dm 3632 (#552)

* dm-3632

Added XR Network dropdown nav in header.

* dm-3632

highlighting/bolding for currently selected menu item

* dm-3632

add some tests for nav.

* dm-3632

update path info based on community/xr-network routes

* dm-3632

PR comments adjustments

* dm-3632

test for top nav communities.

* dm-3632

test for home page.

* dm-3632

test for communities about page.

* dm-3632

* dm-3632

* dm-3632

* dm-3632

* dm-3632

revert tests - not working.

* dm-3632

this test should work..

* dm-3632

slug needs to be 'home'

* dm-3632

change slug back to home.

* dm-3632

Adding path tests for all xr-network pages.

* dm-3632

adding the pages.

* Communities redirect fix (#555)

* Fixed community redirect issue and added a test

* Improved test scenario description

* Trying to fix test on CI

* Hopefully fixed test on CI

Co-authored-by: Brad Johnson <[email protected]>
Co-authored-by: Camille Villa <[email protected]>
Co-authored-by: Camille Villa <[email protected]>
Co-authored-by: bradjohnson92008 <[email protected]>
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.

3 participants