Navigation Menu

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

Disable questionnaires when closing/fixing reports via Open311 #2310

Merged
merged 1 commit into from Oct 25, 2018

Conversation

davea
Copy link
Member

@davea davea commented Oct 25, 2018

Stops questionnaires being sent for reports closed/fixed via Open311.

Not sure if we want this behaviour for situations where a report has been closed/fixed by an admin/council staff user too?

Connects to https://github.com/mysociety/fixmystreet-freshdesk/issues/24.

Please check the following:

  • Is new functionality tested? CodeCov will warn you about the diff coverage, but won’t complain about e.g. new files;
  • Have you updated the changelog? If this is not necessary, put square brackets around this: skip changelog

@ghost ghost assigned davea Oct 25, 2018
@ghost ghost added the Reviewing label Oct 25, 2018
@davea davea requested a review from dracos October 25, 2018 08:49
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I thought this was only for closed reports, I can't see us saying we'd do it for fixed ones? Fixed it makes more sense to still send it, I think, because in that case the questionnaire is reworded to ask if it truly is fixed, and gives the reporter the opportunity to reopen.

Yes, I don't see any reason not to make it apply to all closed reports, however made, in which case you can change Script/Questionnaire.pm to ignore closed_states (and/or mark them as sent) rather than do it at Open311 fetch time and have to spot any other place a state could be set?

@codecov
Copy link

codecov bot commented Oct 25, 2018

Codecov Report

Merging #2310 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2310      +/-   ##
==========================================
+ Coverage   80.28%   80.28%   +<.01%     
==========================================
  Files         187      187              
  Lines       12082    12084       +2     
  Branches     2235     2236       +1     
==========================================
+ Hits         9700     9702       +2     
  Misses       1622     1622              
  Partials      760      760
Impacted Files Coverage Δ
perllib/Open311/GetServiceRequestUpdates.pm 90.8% <100%> (+0.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d20ccf5...b35ce07. Read the comment docs.

@davea
Copy link
Member Author

davea commented Oct 25, 2018

All good points! I must have imagined the fixed requirement, sorry about that. Will force-push a much simpler change shortly.

@davea davea force-pushed the issues/freshdesk/24-fixed-questionnaires branch from 4e11156 to 3fa3954 Compare October 25, 2018 12:04
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👻

@davea davea force-pushed the issues/freshdesk/24-fixed-questionnaires branch from 3fa3954 to b35ce07 Compare October 25, 2018 14:45
@davea davea merged commit b35ce07 into master Oct 25, 2018
@ghost ghost removed the Reviewing label Oct 25, 2018
@davea davea temporarily deployed to github-pages October 25, 2018 14:58 Inactive
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.

None yet

2 participants