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

[WIP] Send messages via writeinpublic #2329

Closed
wants to merge 70 commits into from

Conversation

chrismytton
Copy link
Member

@chrismytton chrismytton commented Nov 15, 2017

Outstanding tasks

  • Use the correct person ID, rather than a hard-coded one
  • Move the WriteInPublic URL, instance ID, username and API key into the config (currently all hard-coded)
  • Redirect to message page after creation
  • Display message replies on the messages page
  • Add message preview step
  • Write some tests for the form and API client
  • Breadcrumbs on individual message page?? ("Write" link current 404s)
  • Embed list of messages into representative profile page (as a tab)? Or ajax it in? [ZA] add a tab on a person pages to show any correspondence on WriteInPublic #2301

Notes to merger

Fixes #2326

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 54.214% when pulling 66f3198 on send-messages-via-writeinpublic into 17f4cf6 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 54.202% when pulling e7093d3 on send-messages-via-writeinpublic into 17f4cf6 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-9.0e-05%) to 54.191% when pulling 1b8277a on send-messages-via-writeinpublic into 17f4cf6 on master.

@mysociety-pusher mysociety-pusher force-pushed the send-messages-via-writeinpublic branch from 1b8277a to ef27218 Compare November 27, 2017 12:17
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 54.14% when pulling ef27218 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 54.142% when pulling 1649cc2 on send-messages-via-writeinpublic into 63942cc on master.

@mysociety-pusher mysociety-pusher force-pushed the send-messages-via-writeinpublic branch from 1649cc2 to 9961380 Compare November 29, 2017 10:09
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 54.142% when pulling 9961380 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 54.142% when pulling 40be645 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 54.151% when pulling 608a305 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 54.153% when pulling c622fa4 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 54.153% when pulling edd8559 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 54.151% when pulling 62fa4d4 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 54.151% when pulling 0ab4364 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 54.151% when pulling 1486f95 on send-messages-via-writeinpublic into 63942cc on master.

@mysociety-pusher mysociety-pusher force-pushed the send-messages-via-writeinpublic branch 2 times, most recently from 0605eb8 to 3a411c3 Compare December 1, 2017 16:40
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 54.148% when pulling 3a411c3 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 54.148% when pulling e465090 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 54.148% when pulling e465090 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 54.158% when pulling eb64675 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 54.158% when pulling e1dc01a on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 54.158% when pulling e1dc01a on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 54.112% when pulling b94c4d4 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.08%) to 54.112% when pulling 0fa5b9e on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 54.094% when pulling 08878b4 on send-messages-via-writeinpublic into 63942cc on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 54.094% when pulling f4af9af on send-messages-via-writeinpublic into 63942cc on master.

chrismytton and others added 17 commits February 1, 2018 18:25
Put everything inside a .person-write-form so it appears at the correct
width.
Check that the recipients form has been filled in before trying to get
cleaned data from it.
This allows us to link directly to the pre-filled draft step by
including a ?person_id parameter in the URL with the id of the person we
want to start writing to.
This links directly to the draft step with the persons name pre-filled.
This stops people from ending up on the draft or preview pages when
there are no recipients.
This was changed in WriteInPublic because the popolo UUID isn't unique
between different datasources, so we need to include the whole URI with
the datasource prefix included, rather than just the UUID.
When sending the message we want to use flash messages to let the user
know whether the action has succeeded or failed.
If the message is sent successfully then the user will be redirected to
the individual message page, so we need to output the flash messages
there.

If there is an error while sending the message then the user gets sent
back to the start of the message sending process, so the flash message
needs to be shown then.

Note: currently if there is an error then the users message is lost,
which isn't ideal, but it's at least _slightly_ better to send them back
to the start of the process rather than showing them a 500 error.
By using a custom exception which wraps the requests exception we make
it easier for the calling code to catch the exception without needing to
import requests.
If there's a problem connecting to writeinpublic then we want to catch
that exception and show an error message to the user.
@mysociety-pusher mysociety-pusher force-pushed the send-messages-via-writeinpublic branch from ccafb43 to 3e40a3f Compare February 1, 2018 18:26
@chrismytton chrismytton changed the base branch from master to add-person-everypolitician_uuid-method February 5, 2018 17:55
@chrismytton chrismytton changed the base branch from add-person-everypolitician_uuid-method to master February 5, 2018 17:55
@chrismytton
Copy link
Member Author

This was split up into separate pull requests (the "merged" PRs referenced above), so the code from this is now on master.

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.

4 participants