Adding non static content #9

Merged
merged 16 commits into from Mar 21, 2013

Projects

None yet

2 participants

@jackfranklin
Contributor

This is the content for the legalisation pages, plus some small tweaks I had to do to allow us to use the content we need on the pages we need them to.

@jackfranklin
Contributor

Added the foreign marriage certificate transaction.

Now left: death, birth, depositing certificates

@JordanHatch JordanHatch was assigned Mar 21, 2013
@jackfranklin
Contributor

@JordanHatch I think this is now ready for checking & then merging.

@JordanHatch
Contributor

I think you'll need to update the tests - I get 15 failures when I run them with these changes.

@jackfranklin
Contributor

Pfft...tests are overrated.

Updated them with the new content - unbelievably slipped my mind to do it previously slap. Now all green.

@JordanHatch JordanHatch and 1 other commented on an outdated diff Mar 21, 2013
app/views/partials/questions/_document_type.html.erb
@@ -3,7 +3,11 @@
<% @transaction.document_types.each do |value,label| %>
<li>
<%= label_tag "transaction_document_type_#{value}" do %>
- <%= radio_button_tag "transaction[document_type]", value %> <%= label %>
+ <%
+ formatted_label = label
+ formatted_label[0] = formatted_label[0].capitalize
+ %>
@JordanHatch
JordanHatch Mar 21, 2013 Contributor

Is this not the same as label.capitalize?

@jackfranklin
jackfranklin Mar 21, 2013 Contributor

Not quite.

The issue is that "Nulla Osta".capitalize == "Nulla osta", but I don't want it to lowercase the "o" in Osta. There may well be a better way to do this.

@JordanHatch
JordanHatch Mar 21, 2013 Contributor

You could use titleize?

@jackfranklin
jackfranklin Mar 21, 2013 Contributor

But "certificate of no impediment" needs to go to "Certificate of no impediment", it shouldn't have each word capitalised. capitalize would work fine if we didn't have to deal with "Nulla Osta".

@JordanHatch
JordanHatch Mar 21, 2013 Contributor

Why do we need to modify the cases for these? Can we not just store the string as the correct case in transactions.yml and use it here without modification?

@JordanHatch JordanHatch commented on an outdated diff Mar 21, 2013
...ansactions/deposit-foreign-marriage/_confirm.html.erb
@@ -1 +1 @@
-<p>The cost for <%= @calculation.item_list %> is &pound;<%= format_money @calculation.total_cost %>
+<p>The cost to deposit <%= pluralize @calculation.document_count, "certificate" %><% if @calculation.postage %> plus postage<% end %> is &pound;<%= format_money @calculation.total_cost %>.
@JordanHatch
JordanHatch Mar 21, 2013 Contributor

Why doesn't this use @calculation.item_list here?

@JordanHatch JordanHatch commented on the diff Mar 21, 2013
lib/transaction_calculator.rb
@@ -48,7 +48,13 @@ def calculate(values)
item_list_order = [:registration, :document, :postage]
return OpenStruct.new(
:total_cost => total_cost,
- :item_list => item_list_order.map {|key| item_list[key] }.join("")
+ :item_list => item_list_order.map {|key| item_list[key] }.join(""),
+ :postage_option => postage_option,
+ :postage_option_label => postage_method.nil? ? "" : postage_method['label'],
+ :document_count => document_count,
+ :postage => postage,
+ :document_type => document_type,
+ :registration_count => registration_count
)
@JordanHatch
JordanHatch Mar 21, 2013 Contributor

It would be cleaner to merge the values hash into the response instead.

@jackfranklin
jackfranklin Mar 21, 2013 Contributor

But then wouldn't we lose the fact that at the beginning of that method we process those values?

document_count = values[:document_count].to_i
postage = values[:postage] == "yes"
etc
@JordanHatch JordanHatch merged commit f557f3b into master Mar 21, 2013
@JordanHatch JordanHatch deleted the adding_non_static_content branch Mar 21, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment