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 related navigation component bugs #205

Merged
merged 2 commits into from
Mar 12, 2018
Merged

Conversation

emmabeynon
Copy link
Contributor

For https://trello.com/c/nGtzTPCu/283-update-government-frontend-to-use-relatednavigation-component

While implementing the related_navigation component in government-frontend we found a couple of bugs, which this PR addresses:

  • Worldwide organisation links were being constructed in the payload used in the component view, but these aren't required.
  • The correct number of mainstream browse pages weren't displaying in the component in cases where a grandparent mainstream browse page is present.

Worldwide Organisations shouldn't be displayed in the related nav, so we remove
this from the helper.
@tijmenb tijmenb had a problem deploying to govuk-publishing-compon-pr-205 March 8, 2018 17:05 Failure
We found a bug whereby grandparent mainstream browse pages weren't being added
to the "topics" part of the `related_navigation` payload. This updates a couple
of methods where the content item's keys weren't being handled correctly and
adds a test to check for the presence of parent and grandparent mainstream
browse pages.
Copy link
Contributor

@steventux steventux left a comment

Choose a reason for hiding this comment

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

This looks good to me, however the heroku build is failing on:

       Bundler Output: Warning: the running version of Bundler (1.15.2) is older than the version that created the lockfile (1.16.1). We suggest you upgrade to the latest version of Bundler by running `gem install bundler`.
       You are trying to install in deployment mode after changing
       your Gemfile. Run `bundle install` elsewhere and add the
       updated Gemfile.lock to version control.
       
       The gemspecs for path gems changed
 !
 !     Failed to install gems via Bundler.
 !
 !     Push rejected, failed to compile Ruby app.
 !     Push failed

I'm not sure what has messed with Gemfile.lock but we either need to fix in another PR, merge and rebase, or tack a commit here to fix it.

@vanitabarrett
Copy link
Contributor

@steventux I've got some other PR's open here which I got to build by adding govuk_publishing_components (5.4.0) to the Gemfile.lock https://github.com/alphagov/govuk_publishing_components/pull/202/files

I think it must have been missing in a previous commit that's now been merged?

@steventux
Copy link
Contributor

@vanitabarrett 👍 so a rebase should fix the issue?

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.

None yet

4 participants