Suggestion feedback and accepting altered suggestions #4659
Conversation
| SubmissionTypes.SUGG_REJECT: _(u'Rejected suggestion from %s', | ||
| author_link)} | ||
| return description_dict.get(self.submission_type, None) | ||
| SubmissionTypes.SUGG_ACCEPT: _( |
There was a problem hiding this comment.
can we break up this method, its really hard to understand like this
ta2-1
force-pushed
the
suggestion_feedback_ui
branch
5 times, most recently
from
5631ddc
to
78b6482
Compare
| @@ -236,7 +238,7 @@ def grouped_entries(self): | |||
| grouped_entries.reverse() | |||
| return grouped_entries | |||
|
|
|||
| @property | |||
| @cached_property | |||
| def submissions(self): | |||
There was a problem hiding this comment.
i think keep this as a @Property that returns a qs, and add a cached_property submission_values or similar that turns it into a regular list
ta2-1
force-pushed
the
suggestion_feedback_ui
branch
2 times, most recently
from
c2d984e
to
1ea7cb5
Compare
|
|
||
| @cached_property | ||
| def comments(self): | ||
| return dict([ |
There was a problem hiding this comment.
can we rename this comment_dict or similar
ta2-1
force-pushed
the
suggestion_feedback_ui
branch
from
a5c6cae
to
337b4e1
Compare
|
|
||
|
|
||
| @pytest.mark.django_db | ||
| def test_reject_suggestion_with_comment(rf, admin): |
There was a problem hiding this comment.
can we make this test for request_users and make sure that the other users get the approp response, probs sim elsewhere
|
Reviewed and tested. gtm. |
| author_link), | ||
| SubmissionTypes.SUGG_REJECT: _(u'Rejected suggestion from %s', | ||
| author_link)} | ||
| SubmissionTypes.SUGG_ACCEPT: _( |
There was a problem hiding this comment.
can we break these out to separate methods/attributes/properties
they are kinda hard to follow atm
ta2-1
force-pushed
the
suggestion_feedback_ui
branch
from
337b4e1
to
e63d869
Compare
| @@ -219,6 +251,29 @@ def submissions(self): | |||
| creation_time=self.object.commented_on) | |||
| .order_by("id")) | |||
|
|
|||
| @cached_property | |||
There was a problem hiding this comment.
im not sure there is much point using a cached_property if we dont do list() on the result
There was a problem hiding this comment.
hi @phlax, it should be cached because we call it twice otherwise. but probs I misunderstood.
There was a problem hiding this comment.
thinking about it you are right, i suppose i just wonder if we should turn it into a list
If suggestion is provided in submit request data we accept this suggestion before submitting. When we calculate updated_fields during submission we should cout unit.state as already TRANSLATED.
ta2-1
force-pushed
the
suggestion_feedback_ui
branch
from
1df2840
to
26a28bb
Compare
It allows to refer to `this` in the handlers instead of `PTL.editor`.
ta2-1
force-pushed
the
suggestion_feedback_ui
branch
from
0e60047
to
c099ab4
Compare
|
Very well done, @ta2-1! This is GTM from my side. |
|
great work, thanks @ta2-1 |
|
and @julen ! |
|
Thanks for your help @julen! |
This PRs addresses #2596 and #4629 issues.
@dwaynebailey, @iafan: you have discussed how we should credit author of suggestion which was accepted with alteration. This PR splits accepting suggestion with alteration into two actions:
It was the quickest possible way how I could do it. I realise that such solution doesn't address all concerns but I suggest to consider them after we land this.