maiera / gde-app

Apache License 2.0
22 stars 9 forks source link

activity_record.list can't be ordered by total_impact #168

Closed SmokyBob closed 9 years ago

SmokyBob commented 9 years ago

For #111 is required to get the top 100 activities by impact, but total_impact is not one of the fields enabled for querying.

I think its due to the fact that total_impact is not a real field but a function.

Is it possible to change it to be like date_created?

patt0 commented 9 years ago

@SmokyBob I think the index is based on post_date not date_created, at least that is the index that has been created

@Scarygami currently total_impact is an EndpointAliasProperty, we could change it to a EndpointComputedProperty which should enable us to query. Have you seen any working examples of that?

Scarygami commented 9 years ago

ComputerProperty should work since it's computed on put() and stored to the datastore, so ordering/filtering should work. Will do some testing.

SmokyBob commented 9 years ago

@patt0 yes the index is no post_date, but I've tried searching for date_created and worked, while I got a 503 if I tried to use the total_impact.

P.S. Low priority issue as for #111 we still load all the activity_record in a period for the other charts so I simply filter the results of the date filtered request

Scarygami commented 9 years ago

Sorry, was already looking into it when you posted about low priority :) https://github.com/maiera/gde-app/pull/170

patt0 commented 9 years ago

I have:

have a look at the data both with the api and console, let me know what you think of the following #bug

https://appengine.google.com/datastore/explorer?submitted=1&app_id=s~elite-firefly-737&show_options=no&version_id=3.380551203244046826&kind=ActivityRecord&query=SELECT+*+FROM+Account

https://apis-explorer.appspot.com/apis-explorer/?base=https://elite-firefly-737.appspot.com/_ah/api#p/gdetracking/v1.0b2/gdetracking.activity_record.list

The datastore has the proper values (based on the current formula) in the total_impact column but when I use the api to list ActivityRecords we get a result of 1 for each total impact and the def() code seems to be run ( see logs )

changes I have uploaded (task needs to be transformed to a cron, so we get 10 minutes to execute, currently I simply called on post on the task url) https://github.com/patt0/gde-app/commit/26980a89bdec3be939a0541b7edf613bdaa65f3f

Scarygami commented 9 years ago

I know what the issue is there (basically a bug in the endpoints library that can be worked around). Currently not at a PC, will post a fix and more details later.

Scarygami commented 9 years ago

See https://github.com/patt0/gde-app/pull/2

patt0 commented 9 years ago

Brilliant thank you @Scarygami

Patrick Martinent

On Thu, Dec 4, 2014 at 7:05 PM, Gerwin Sturm notifications@github.com wrote:

See patt0#2 https://github.com/patt0/gde-app/pull/2

— Reply to this email directly or view it on GitHub https://github.com/maiera/gde-app/issues/168#issuecomment-65631901.

SmokyBob commented 9 years ago

Got around the web app pointing to the staging backend with the new weight, really linking the weighted impact, especially for the top 100 impactful activities

patt0 commented 9 years ago

We are serving an index so you can get the activities from each gde, directly sorted based on total_impact.

I am ready to push the feature to Production including a recalc of total impact when you give me a go ahead.

The update will also put regular back ups in place.

Patrick Martinent

On Fri, Dec 5, 2014 at 3:16 AM, Mauro Solcia notifications@github.com wrote:

Got around the web app pointing to the staging backend with the new weight, really linking the weighted impact, especially for the top 100 impactful activities

— Reply to this email directly or view it on GitHub https://github.com/maiera/gde-app/issues/168#issuecomment-65709446.

SmokyBob commented 9 years ago

@patt0 Please go with the push to prod as everything looks good to me.

patt0 commented 9 years ago

Have a look - looks like its all done.

Patrick Martinent

On Fri, Dec 5, 2014 at 1:54 PM, Mauro Solcia notifications@github.com wrote:

@patt0 https://github.com/patt0 Please go with the push to prod as everything looks good to me.

— Reply to this email directly or view it on GitHub https://github.com/maiera/gde-app/issues/168#issuecomment-65759958.

SmokyBob commented 9 years ago

Yep everything works. Thanks