Threads to be sent to municipality #741

PetrDlouhy · 2017-12-03T11:45:55Z

This is unfinished functionality prototype meant to be discussed and finished before pulling.

This PR adds new button "Send to the municipality" which adds new message thread with parameter that it should be send to the municipality. We are working in cooperation with Prague city council on this function which will send automatically Issue data to the official portal http://zmente.to.
The new thread form is prefilled with the issue title and description, the user can modify this or leave it.
This function is available only for members of the groups to prevent inexperienced users from sending low quality issues.

There some things, that has to be solved before pulling:

The administrator should be able to add new external services user can choose from.
If there is no such service, user should not see the button.

Please comment, what do you think about this function.

nikolai-b · 2017-12-03T20:40:01Z

Wow, that is an exciting possibility. I think it is really for @mvl22 to deal with as this is not a coding issue so I can't comment.

mvl22 · 2017-12-03T22:01:09Z

Thanks for this, Petr!

There seem to be hard-coded references to the specific service (zmente.to) in the code - I think this stuff should be generalised, perhaps in the form of a driver file which implements particular services and then the settings contain a drop down of the available drivers (albeit only one at present).

On a policy question, if any member can submit an issue externally, what is required to ensure that the submission represents the considered view of the members of the discussion thread as a whole? For instance, couldn't a new user come along, and submit something wildly different to the consensus view?

PetrDlouhy · 2017-12-05T16:43:11Z

@mvl22 For now, this only sets submit_external parameter of message thread to the service identifier value (zmenteto). Then the marked threads can be filtered in the API. I solved the sending from Cyclescape API to Změňte.to API by external script. Although it might be desired to include external service senders to the code in the future, I went the simplest way for me now.

I would like to add a possibility to configure possible services in the site config. My idea is, that user only fills service name and service identifier (and maybe other identifiers in the future) to the table. If the table is empty, no Send to municipality button is shown.
But I am not quite sure how to do this. The only way I can imagine is changing the submit_external parameter to foreign key to the service table.

Změňte.to is a service opened to the wide public, so opening this possibility to the group member is more restrictive than the original service. So for my use-case, this is OK and I would like to keep this simple. If anyone wants to add some service that requires more restrictive approach, he can add it by some settings of the services.

PetrDlouhy · 2017-12-14T16:52:05Z

I updated this PR and I consider it to be feature complete now. I changed the thread parameter to external_service foreign key and added external services configuration to admin.

So now there is no reference to any particular service and everything is done by configuration in admin. If no service is added in admin, no "Send to municipality" button is displayed, so nothing changes.

Please review.

PetrDlouhy · 2017-12-19T11:32:26Z

I rebased this to cs/staging

nikolai-b · 2017-12-19T15:37:31Z

Thanks @PetrDlouhy I don't have time to review this one properly yet and I'd like to do it after the next release, do you mind it waiting here for a bit?

Thanks

PetrDlouhy · 2017-12-20T07:52:45Z

I would like, if it got there, but I cannot force you :-)

PetrDlouhy · 2018-07-31T11:33:31Z

@nikolai-b When will you have time to review this? If you write me in advance, I could rebase this before your review.

mvl22 · 2018-08-01T12:14:36Z

I could rebase this before your review.

Yes, if you could update to the latest code, that will speed up review - I see there is a conflict noted for instance.

PetrDlouhy · 2018-09-07T10:24:33Z

@mvl22 OK, rebased.

nikolai-b

I'm unsure about the UI change.

Which creates a normal new thread and marks it as external which presumably your script then is able to pull in from the API. Why not just add it as a drop down (but allow it to be blank) whenever a new thread is created?

Other than that the code is fine, I've made a few minor comments. Thank you @PetrDlouhy !

nikolai-b · 2018-09-17T17:52:01Z

app/controllers/admin/external_services_controller.rb

+
+ def create
+ @external_service = ExternalService.new(permitted_params)
+ puts @external_service


I'll remove this after merging.

nikolai-b · 2018-09-17T17:56:08Z

app/views/home/user_profile_template.html.erb

- <div class="permissions">shared</div>
+ <div class="thread-parameters">
+ <div class="permissions">shared</div>
+ </div>


I think this whole code is left over from the initial design (i.e. it isn't used live). @mvl22 was there a reason the original designs were kept as part of this repository? Can we delete it? They will exists in git if we do ever need to refer to them.

was there a reason the original designs were kept as part of this repository

Because the design has never actually been fully implemented yet! That said, I am trying to get round to commissioning a fresh new design that mops up piles of usability issues.

nikolai-b · 2018-09-17T17:57:29Z

app/views/issues/show.html.haml

@@ -49,6 +49,10 @@
 %aside#sidebar.wide
 - if permitted_to? :create, :issue_message_threads
 = link_to t(".new_thread", count: @issue.threads.count), new_issue_thread_path(@issue), class: "btn-green", rel: "#overlay"
+ - if current_user and current_user.groups.present?


and is a trick in ruby and doesn't do things sensibly, I'll change to && when I merge

nikolai-b · 2018-09-17T17:58:56Z

app/models/external_service.rb

+ "#{id}-#{short_name}"
+ end
+
+ protected


Any reason this is here?

nikolai-b · 2018-09-17T18:12:26Z

db/migrate/20171212101253_add_submit_external_to_message_thread.rb

@@ -0,0 +1,9 @@
+class AddSubmitExternalToMessageThread < ActiveRecord::Migration
+ def change
+ add_column :message_threads, :external_service_id, :integer


If we use
add_reference :message_threads, :external_service, foreign_key: true
it adds and index and preserves referential integrity (i.e. we can't have a thread referencing a non-existent external service.

nikolai-b · 2018-09-17T18:13:23Z

db/migrate/20171212101253_add_submit_external_to_message_thread.rb

+ create_table :external_services do |t|
+ t.string :name, null: false
+ t.string :short_name, null: false
+ end


We want a uniqueness constraint on :short_name
add_index :external_services, :short_name, unique: true

nikolai-b · 2018-09-17T18:15:39Z

db/migrate/20171212101253_add_submit_external_to_message_thread.rb

+ t.string :short_name, null: false
+ end
+ end
+end


In pulls it helps if you also run rake db:migrate and commit the schema changes.

nikolai-b · 2018-09-17T18:24:01Z

app/models/external_service.rb

+ has_many :threads, class_name: 'MessageThread', inverse_of: :external_service
+
+ validates :name, presence: true, uniqueness: true
+ validates :short_name, presence: true, uniqueness: true, subdomain: true


What is this? As in what is the difference between the name and short name? Why not just use the name?

nikolai-b · 2018-09-17T18:46:36Z

app/controllers/issue/message_threads_controller.rb

+ end
+ @message = @thread.messages.build
+ @message.body = issue.description
+ @thread.title = issue.title


I'm not sure about setting the thread title equal to the issue title.
I think it should be:
Issue: New housing will be built
Thread: Increased congestion on the road
Thread: Cycle Parking on the New housing application is poor

etc.

See #43 for context.

mvl22 · 2018-09-17T18:46:42Z

which adds new message thread with parameter that it should be send to the municipality. We are working in cooperation with Prague city council on this function which will send automatically Issue data to the official portal http://zmente.to.

To be clear - is this purely copying the initial problem report to the municipality?

nikolai-b · 2018-09-17T18:54:59Z

@mvl22 is #741 (comment) addressed to me or @PetrDlouhy ? I think the Send To Municipality button will make a new thread and mark it as such so the thread is sent to the external service.

mvl22 · 2018-09-17T19:28:35Z

@mvl22 is #741 (comment) addressed to me or @PetrDlouhy?

To Petr.

I think the Send To Municipality button will make a new thread and mark it as such so the thread is sent to the external service.

Ah, this is not what I was expecting. Peter - why does a report also create a new thread? Can these not be separated into "Create a new thread" (which already exists) and "Send to municipality" (as to be added)? I think it would be clearest to keep these two things completely separate.

PetrDlouhy force-pushed the feature/municipality-threads branch 4 times, most recently from 9f6d9b0 to 63af588 Compare December 14, 2017 16:29

PetrDlouhy changed the title ~~(WIP) Threads to be sent to municipality~~ Threads to be sent to municipality Dec 14, 2017

PetrDlouhy force-pushed the feature/municipality-threads branch 2 times, most recently from 5d6e8ed to 2b1891d Compare December 19, 2017 11:32

PetrDlouhy force-pushed the feature/municipality-threads branch from 2b1891d to e69f05c Compare December 21, 2017 10:24

PetrDlouhy force-pushed the feature/municipality-threads branch 2 times, most recently from 5ca9473 to b291487 Compare January 2, 2018 09:16

nikolai-b force-pushed the staging branch from a1bf6ea to 1fc0216 Compare February 19, 2018 21:28

PetrDlouhy force-pushed the feature/municipality-threads branch from b291487 to 3d82b80 Compare September 7, 2018 10:23

PetrDlouhy added 2 commits September 7, 2018 12:42

add send to municipality button

1c10944

add external_service parameter to thread API

d63f000

PetrDlouhy force-pushed the feature/municipality-threads branch from 3d82b80 to d63f000 Compare September 7, 2018 10:43

nikolai-b reviewed Sep 17, 2018

View reviewed changes

nikolai-b force-pushed the staging branch from 20237fe to d380276 Compare December 14, 2018 20:38

nikolai-b force-pushed the staging branch from ddf177e to f00506e Compare April 14, 2020 11:18

nikolai-b force-pushed the staging branch 2 times, most recently from 0a772ab to 75e28de Compare November 15, 2022 22:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Threads to be sent to municipality #741

Threads to be sent to municipality #741

PetrDlouhy commented Dec 3, 2017 •

edited

nikolai-b commented Dec 3, 2017

mvl22 commented Dec 3, 2017 •

edited

PetrDlouhy commented Dec 5, 2017

PetrDlouhy commented Dec 14, 2017

PetrDlouhy commented Dec 19, 2017

nikolai-b commented Dec 19, 2017

PetrDlouhy commented Dec 20, 2017

PetrDlouhy commented Jul 31, 2018

mvl22 commented Aug 1, 2018 •

edited

PetrDlouhy commented Sep 7, 2018

nikolai-b left a comment •

edited

nikolai-b Sep 17, 2018

nikolai-b Sep 17, 2018

mvl22 Sep 17, 2018

nikolai-b Sep 17, 2018

nikolai-b Sep 17, 2018

nikolai-b Sep 17, 2018

nikolai-b Sep 17, 2018

nikolai-b Sep 17, 2018

nikolai-b Sep 17, 2018

nikolai-b Sep 17, 2018

mvl22 commented Sep 17, 2018

nikolai-b commented Sep 17, 2018

mvl22 commented Sep 17, 2018

Threads to be sent to municipality #741

Are you sure you want to change the base?

Threads to be sent to municipality #741

Conversation

PetrDlouhy commented Dec 3, 2017 • edited

nikolai-b commented Dec 3, 2017

mvl22 commented Dec 3, 2017 • edited

PetrDlouhy commented Dec 5, 2017

PetrDlouhy commented Dec 14, 2017

PetrDlouhy commented Dec 19, 2017

nikolai-b commented Dec 19, 2017

PetrDlouhy commented Dec 20, 2017

PetrDlouhy commented Jul 31, 2018

mvl22 commented Aug 1, 2018 • edited

PetrDlouhy commented Sep 7, 2018

nikolai-b left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvl22 commented Sep 17, 2018

nikolai-b commented Sep 17, 2018

mvl22 commented Sep 17, 2018

PetrDlouhy commented Dec 3, 2017 •

edited

mvl22 commented Dec 3, 2017 •

edited

mvl22 commented Aug 1, 2018 •

edited

nikolai-b left a comment •

edited