-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add polygon geographies to Budgets' map #3907
Conversation
ea0b5c3
to
38a6f47
Compare
13fcd0e
to
46119e8
Compare
|
||
expect(page).not_to have_content "Geography created successfully!" | ||
expect(page).to have_css(".is-invalid-label", text: "Geojson") | ||
expect(page).to have_css("small.form-error", text: "The GeoJSON provided does not follow the correct format. " \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [116/110] (https://rubystyle.guide#max-line-length)
expect(page).not_to have_content "Geography created successfully!" | ||
expect(page).to have_css(".is-invalid-label", text: "Geojson") | ||
expect(page).to have_css("small.form-error", text: "The GeoJSON provided does not follow the correct format. " \ | ||
'It must follow the "Polygon" or "MultiPolygon" type format.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [117/110] (https://rubystyle.guide#max-line-length)
|
||
expect(page).not_to have_content "Geography updated successfully!" | ||
expect(page).to have_css(".is-invalid-label", text: "Geojson") | ||
expect(page).to have_css("small.form-error", text: "The GeoJSON provided does not follow the correct format. " \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [116/110] (https://rubystyle.guide#max-line-length)
expect(page).not_to have_content "Geography updated successfully!" | ||
expect(page).to have_css(".is-invalid-label", text: "Geojson") | ||
expect(page).to have_css("small.form-error", text: "The GeoJSON provided does not follow the correct format. " \ | ||
'It must follow the "Polygon" or "MultiPolygon" type format.') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [117/110] (https://rubystyle.guide#max-line-length)
46119e8
to
ff800ec
Compare
ff800ec
to
198c4d6
Compare
6fb9c63
to
d956c40
Compare
@@ -59,7 +59,7 @@ def budget_heading_params | |||
end | |||
|
|||
def allowed_params | |||
valid_attributes = [:price, :population, :allow_custom_content, :latitude, :longitude, :max_ballot_lines] | |||
valid_attributes = [:price, :population, :allow_custom_content, :latitude, :longitude, :max_ballot_lines, :geozone_id] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [124/110] (https://rubystyle.guide#max-line-length)
app/models/geozone.rb
Outdated
@@ -4,7 +4,9 @@ class Geozone < ApplicationRecord | |||
has_many :proposals | |||
has_many :debates | |||
has_many :users | |||
has_many :headings, class_name: "Budget::Heading" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rails/HasManyOrHasOneDependent: Specify a :dependent
option. (https://rails.rubystyle.guide#has_many-has_one-dependent-option)
That way it'll be easier to refactor it.
This way it'll be easier to test it.
This way we simplify the header and it will be easier to add more code and tests.
Not sure why it was added in commit 9fb5019, but it made the table look funny on some screens, particularly after adding the extra field we're about to add.
This way we remove some logic from the (huge) investments controller class.
5b3066e
to
3e9339d
Compare
|
||
render_inline component | ||
|
||
expect(page).not_to have_select "Scope of operation", options: ["All City", "Under the sea", "Above the skies"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [117/110] (https://rubystyle.guide#max-line-length)
geozones = [ | ||
create(:geozone, :with_geojson, name: "GeoJSON", external_code: "1", census_code: "2"), | ||
create(:geozone, :with_html_coordinates, name: "HTML", external_code: "3", census_code: "4"), | ||
create(:geozone, :with_geojson, :with_html_coordinates, name: "With both", external_code: "6", census_code: "7"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [121/110] (https://rubystyle.guide#max-line-length)
end | ||
|
||
trait :with_geojson do | ||
geojson { '{ "geometry": { "type": "Polygon", "coordinates": [[-0.117,51.513],[-0.118,51.512],[-0.119,51.514]] } }' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [123/110] (https://rubystyle.guide#max-line-length)
spec/system/admin/geozones_spec.rb
Outdated
describe "polygons" do | ||
before { Setting["feature.map"] = true } | ||
|
||
let!(:geojson) { '{ "geometry": { "type": "Polygon", "coordinates": [[-0.1,51.5],[-0.2,51.4],[-0.3,51.6]] } }' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [116/110] (https://rubystyle.guide#max-line-length)
@@ -1698,6 +1698,27 @@ def investments_order | |||
end | |||
end | |||
|
|||
scenario "Shows the polygon associated to the current heading" do | |||
triangle = '{ "geometry": { "type": "Polygon", "coordinates": [[-0.1,51.5],[-0.2,51.4],[-0.3,51.6]] } }' | |||
rectangle = '{ "geometry": { "type": "Polygon", "coordinates": [[-0.1,51.5],[-0.2,51.5],[-0.2,51.6],[-0.1,51.6]] } }' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [123/110] (https://rubystyle.guide#max-line-length)
The `map` class is applied to the map element by LeafletJS; using it in the container led to hacks like adding an `inline` class to fix the fact that the container was using the `height` rule of the `.map` elements. Even though we don't add styles for them, I'm adding the `budgets-map` and `budget-investments-map` HTML classes so these elements can still be easily selected with CSS and JavaScript.
This check isn't necessary since commit 7e3dd47, since now we check that the budget is present before creating the components which call this method.
Just like we do in pretty much every section in the admin area.
Note that in the budgets wizard test we now create district with no associated geozone, so the text "all city" will appear in the districts table too, meaning we can't use `within "section", text: "All city" do` anymore since it would result in an ambiguous match. Co-Authored-By: Julian Herrero <microweb10@gmail.com> Co-Authored-By: Javi Martín <javim@elretirao.net>
Note that, in this case, we aren't binding a popup to the polygon because the link would point to the same page we're already in.
3e9339d
to
a9029be
Compare
|
||
render_inline component | ||
|
||
expect(page).to have_select "Scope of operation", options: ["All city", "Under the sea", "Above the skies"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/LineLength: Line is too long. [113/110] (https://rubystyle.guide#max-line-length)
Huge thanks to @MatheusMiranda and @microweb10 for this pull request! 🙏 Sorry it took so long, but we finally got there 🎉. |
References
Objectives
Add feature to create Geographies. This geographies may have GeoJSON's attributtes and be plotted as polygon on Budgets' map.
Visual Changes
On Administration section, at Settings area, there is now an option to Manage Geographies.
It is possible to create Geographies, defining its color and the Headings related to this geography. Also it's possible to paste a GeoJSON file to define polygon to be drawed.
The name of related headings are shown for each geography. "No related Headings" is shown for geographies without related headings.
The name of related geography is shown for each heading. "No related geography" is shown when there is no associated geography;
Notes