GoogleDeveloperExperts / experts-app-backend

Future home for the backend source of the GDE Tracking App
Apache License 2.0
15 stars 6 forks source link

Consistence problem on delete activity posts #29

Closed LostInBrittany closed 8 years ago

LostInBrittany commented 8 years ago

Currently there is a nasty problem when we delete Activity Posts: we delete the Activity Posts, but we don't delete its reference in its parent Activity Record's activity_posts field. And then, when we request the said Activity Record, it tries to access to the Activity Post to calculate the metrics, and as the post doesn't exist anymore, we get an internal server error.

I see three possible solutions :

  1. In the Delete Activity Post endpoint we modify also the parent Activity Record to delete the Activity Post reference
  2. Not to use the said reference anymore, as we don't need it really... Right now we calculate the metrics like this:

    def metric_reached(self):
       total_reached = 0
    
       if len(self.activity_posts) == 0:
           return total_reached
    
       for post_id in self.activity_posts:
           post_key = ndb.Key(ActivityPost, int(post_id))
           activity_post = post_key.get()
           total_reached += activity_post.metric_reached
       return total_reached

    We could instead directly query the Activity post to get those whose the parent is the current Activity Record:

    def metric_reached(self):
       total_reached = 0
    
       for activity_post in ActivityPost.query(ActivityPost.id == self.id) :
           total_reached += activity_post.metric_reached
       return total_reached 
    
  3. Model Activity Posts as Structured Properties If I understand correctly, every Activity Post is always associated to one and only one Activity Record, i.e. they are not full-fledged entities.

    I'm not an expert on Google API Engine, but according to the reference they seem a good candidate for be modelled as Structured Properties instead of Entities.

    That would make some things way easier. If Activity Post were Structured Properties instead of Entities, the consistence problem would disappear by itself...

patt0 commented 8 years ago
  1. I think I solved this in the current code deployed this morning after your post.
  2. We can think about the benefit of accessing the posts independently for batch processing of metric updates and whether going through Activity Records is a pain.

Patrick Martinent

On 17 August 2016 at 04:18, Horacio Gonzalez notifications@github.com wrote:

If I understand correctly, every Activity Post is always associated to one and only one Activity Record, i.e. they are not full-fledged entities.

I'm not an expert on Google API Engine, but according to the reference https://cloud.google.com/appengine/docs/python/ndb/entity-property-reference#structured they seem a good candidate for be modelled as Structured Properties instead of Entities.

That would make some things way easier. Right now wer have a nasty consistence problem: when we delete an Activity Record, we must delete the associated activity posts (this is done in the current version), and when we delete an Activity Post we should delete the reference to the post in the parent Activity Record (and it isn't done in current version, generating nasty errors 500 when we request again the Activity Record...).

If Activity Post were Structured Properties instead of Entities, the consistence problem would disappear by itself...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/GoogleDeveloperExperts/experts-app-backend/issues/29, or mute the thread https://github.com/notifications/unsubscribe-auth/ADoQJIiaFUJ-MtVxi3jCWrEEviqRimUbks5qgj5OgaJpZM4Jl7YW .

LostInBrittany commented 8 years ago

Yeas, the problem was solved, I forgot to close this one :)

About accessing the posts independently, yeah, it could be a need. But with the Structured properties it could be done almost as easily.

On Wed, Aug 17, 2016 at 8:30 PM, Patrick Martinent <notifications@github.com

wrote:

  1. I think I solved this in the current code deployed this morning after your post.
  2. We can think about the benefit of accessing the posts independently for batch processing of metric updates and whether going through Activity Records is a pain.

Patrick Martinent

On 17 August 2016 at 04:18, Horacio Gonzalez notifications@github.com wrote:

If I understand correctly, every Activity Post is always associated to one and only one Activity Record, i.e. they are not full-fledged entities.

I'm not an expert on Google API Engine, but according to the reference https://cloud.google.com/appengine/docs/python/ndb/ entity-property-reference#structured they seem a good candidate for be modelled as Structured Properties instead of Entities.

That would make some things way easier. Right now wer have a nasty consistence problem: when we delete an Activity Record, we must delete the associated activity posts (this is done in the current version), and when we delete an Activity Post we should delete the reference to the post in the parent Activity Record (and it isn't done in current version, generating nasty errors 500 when we request again the Activity Record...).

If Activity Post were Structured Properties instead of Entities, the consistence problem would disappear by itself...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://github.com/GoogleDeveloperExperts/experts-app-backend/issues/29 , or mute the thread https://github.com/notifications/unsubscribe-auth/ADoQJIiaFUJ- MtVxi3jCWrEEviqRimUbks5qgj5OgaJpZM4Jl7YW .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/GoogleDeveloperExperts/experts-app-backend/issues/29#issuecomment-240503879, or mute the thread https://github.com/notifications/unsubscribe-auth/AAsVzFB5fjeAfx7g2D6MLyQsdfsXzA1Aks5qg1MxgaJpZM4Jl7YW .