[Predator] Refactor ToPublishResult and fix keyerror 'found'

appengine/findit/model/crash/cracas_crash_analysis.py

 # Copyright 2016 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 from google.appengine.ext import ndb
 
 from model.crash.chrome_crash_analysis import ChromeCrashAnalysis
 
 
 class CracasCrashAnalysis(ChromeCrashAnalysis):
   """Represents an analysis of a Chrome crash on Cracas."""
-  pass
+
+  def ProcessResultForPublishing(self, result):  # pragma: no cover.
+    """Cracas specific processing of result data for publishing."""
+    # TODO(katesonia) Add feedback page for Cracas after Cracas integration.

[stgao, 2016/11/23 23:51:14]

Why it is to be added here? Shouldn't model just take care of data
saving/updating/retrieving? Why we want it to deal with UI layer (url to
feedback page)?

[Sharu Jiang, 2016/11/24 00:13:38]

The previous ``ToPublishResult`` is in the publish pipeline, however it has been
moved to CrashAnalysis in that major refactor, I thought you agreed on this
refactor?

I just split the client specific part in ToPublishResult to subclasses.

If you'd prefer, I can move the ``ToPublishResult`` back to publish pipeline and
let FinditFor* to deal with this processing.

[stgao, 2016/11/29 18:44:35]

This is good. I don't have concern on this.
My concern is the TODO added here. That's why the comment is at line #15 instead
of line #13, and the wording is "to be added".

[Sharu Jiang, 2016/11/29 20:49:43]

Ok, sorry, I misunderstood your comment. Ｔhe url to feedback link is like meta
data added to the model result to publish to client. I don't think it actually
dealt with the UI layer．

[stgao, 2016/11/30 04:35:07]

So where is the URL from/defined? How the code should be split into the MVC
design pattern and the dependency among them?

[Sharu Jiang, 2016/11/30 18:14:07]

If this is about where we should insert the feedback url to result for
publishing:

I have concern mentioned in the thread, if we move this part to crash_pipeline
we would have if
client == FRACAS..., which I think it's the thing we want to avoid. If we move
it to FinditFor*, it will be a little wired that we have some processing in
CrashAnalysis model and some in FinditFor... class.

I am a little confused, this may deal with url but it is one kind of processing
just as ``ToPublishableResult`` in CrashAnalysis. If we are to move it, where
should this function stay?

If this is all about the TODO:
I rephrased it and hope it's more clear that this function won't deal with the
url but just insert this url to result.

[stgao, 2016/11/30 18:56:06]

The wording is fine, but my concern is what is to be done.

To be more explicit, I have a question here: why the url defined for a handler
in the controller layer is added to the model layer as in
model/crash/fracas_crash_analysis.py below? Isn't it a violation of the MVC
pattern? Should this be fixed?

Are there alternatives to add the url from the controller layer like the publish
pipeline?

[Sharu Jiang, 2016/11/30 19:26:05]

I think move all the ToPublishableResult to FinditFor* can solve all concerns.
If we all agree on this, I will go ahead and move it.

+    return result