Improve budget investment form #2280
Conversation
config/locales/es/budgets.yml
Outdated
| @@ -58,6 +58,7 @@ es: | |||
| map_location: "Ubicación en el mapa" | |||
| map_location_instructions: "Navega por el mapa hasta la ubicación y coloca el marcador." | |||
| map_remove_marker: "Eliminar el marcador" | |||
| location: "Información adicional de la localización" | |||
There was a problem hiding this comment.
Maybe to be consistent with other spanish texts around locations the word localización should be changed for ubicación as that's what the map uses 3 times on title/subtitle/checkbox
There was a problem hiding this comment.
If external_url won't be used anymore from the form, maybe its a good idea to also remove it from the permitted params at https://github.com/consul/consul/blob/master/app/controllers/budgets/investments_controller.rb#L106
Also... 🤔 its true we can't destroy the column from the database as many forks have values for it ... but maybe adding a rake that "concats" existing values at the end of the "description" attribute with some html tags... could allow us to remove it in two releases in the future?
| @@ -311,7 +311,6 @@ def investments_order | |||
| select 'Health: More hospitals', from: 'budget_investment_heading_id' | |||
| fill_in 'budget_investment_title', with: 'Build a skyscraper' | |||
| fill_in 'budget_investment_description', with: 'I want to live in a high tower over the clouds' | |||
| fill_in 'budget_investment_external_url', with: 'http:https://http:https://skyscraperpage.com/' | |||
There was a problem hiding this comment.
I can see 3 more budget_investment_external_url occurences in the codebase 🤔 in two more tests:
https://github.com/consul/consul/blob/master/spec/features/management/budget_investments_spec.rb#L35
https://github.com/consul/consul/blob/master/spec/features/emails_spec.rb#L372
That management one... maybe there is a form for investments at management section ?
There was a problem hiding this comment.
Yes, in management panel also there is a new budget investment form with this field
|
Maybe To completly deprecate the usage of the attribute |
MariaCheca
force-pushed
the
2277-budgets_new_form
branch
5 times, most recently
from
272892d
to
ad3850b
Compare
| @@ -51,12 +51,6 @@ | |||
| </div> | |||
| <% end %> | |||
|
|
|||
| <% if investment.external_url.present? %> | |||
There was a problem hiding this comment.
But... we want existing budgets with external_url's but no documents to still show those values right?
There was a problem hiding this comment.
Yes, we want to disallow this input field, but maintain the existing links 😌
MariaCheca
force-pushed
the
2277-budgets_new_form
branch
from
ad3850b
to
7a30c38
Compare
MariaCheca
force-pushed
the
2277-budgets_new_form
branch
from
7a30c38
to
cd53884
Compare
…form Budgets new form
The input was removed via consuldemocracy#2280 but due to conflict resolution, it was accidentally re-introduced. This commit removes it altogether as well as its associated I18n keys
Where
What
external_linkinput in budget investment form. The database column will remain for previous budgets where the attribute was used, as it could be useful at some point, or just to keep all the original data as it was introduced.locationinput label.Screenshots
Test
As usual.