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

Inlining review and fact check activity fields #253

Merged
merged 4 commits into from
Sep 10, 2014

Conversation

vinayvinay
Copy link
Contributor

as part of a larger change we inlined all activity fields with the edition form, so that activities and edition changes get submitted in one go. we missed inlining review and fact check activities. so this covers up for that.

also removed an unnecessary template programmes/_review.html.erb, and fixed integrations tests failing due to poltergeist getting confused with overlapping elements.

@@ -1,2 +1,6 @@
<% render_activity_forms ||= false %>
Copy link
Contributor

Choose a reason for hiding this comment

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

styleguide says booleans shouldn't be initialised with ||=. How about render_activity_forms = false unless defined?(:render_activity_forms) instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch! amended in 7649d01

Vinay Patel added 3 commits September 10, 2014 11:07
www.agileplannerapp.com/boards/173808/cards/5629

... with edition form. these were left out earlier,
causing review activities to not save edition changes.

part of a larger change:

#251
this template is not being used anywhere. instead
shared/_review gets rendered. looks like it was
left behind when common templates were moved to
the shared folder.
Poltergeist is seeing '.help-block' overlapping
the 'Save' button, hence can't click it without
a `.trigger('click')` on that element.

This needs to be followed by an assertion that
waits for the edition to get saved, or else
capybara jumps to the next statement.
@vinayvinay vinayvinay force-pushed the save-and-transition-in-on-go-feedback branch from 9e50049 to 19f931a Compare September 10, 2014 11:07
@vinayvinay
Copy link
Contributor Author

i've renamed EVENTS to ACTIONS and created a helper to prevent passing locals around in ec74737. thanks for the suggestions @tekin.

@vinayvinay vinayvinay force-pushed the save-and-transition-in-on-go-feedback branch from ec74737 to b79c851 Compare September 10, 2014 11:35
.. and moving locals logic into a helper,
so that we don't have to pass locals around
to detect the diff view.
@vinayvinay vinayvinay force-pushed the save-and-transition-in-on-go-feedback branch from b79c851 to a17e7b0 Compare September 10, 2014 11:37
@tekin
Copy link
Contributor

tekin commented Sep 10, 2014

👍

vinayvinay added a commit that referenced this pull request Sep 10, 2014
…edback

Inlining review and fact check activity fields
@vinayvinay vinayvinay merged commit e0df429 into master Sep 10, 2014
@vinayvinay vinayvinay deleted the save-and-transition-in-on-go-feedback branch September 10, 2014 12:37
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