Can't get it to log unique impressions #105

Closed
Linuus opened this Issue Aug 16, 2013 · 2 comments

Comments

Projects
None yet
2 participants
@Linuus
Contributor

Linuus commented Aug 16, 2013

Hi

Despite the performance problems discussed in my other issue here:
#94

I will give impressionist a try again in a new project. However, there are some problems.

I can't get impressionist to keep unique impressions in the counter cache.

I have this in my controller:

def show
  @photo = Photo.find(params[:id]).decorate
  impressionist @photo
end

Photo model:

is_impressionable counter_cache: true, unique: true

Still, it logs a new impression in my counter cache (called impressions_count) every time I refresh the page. Shouldn't it be filtering on the IP-adress by default when setting unique: true?

I'm using Rails 4 and impressionist 1.4.5

@Linuus

This comment has been minimized.

Show comment
Hide comment
@Linuus

Linuus Aug 16, 2013

Contributor

Something seems really off here...

If I run: photo.impressionist_count(:filter => :session_hash) it counts all impressions, although I can see they have the same session_hash.

I get the same result with filter: :ip_address

The SQL generated looks like this:
SELECT COUNT("impressions"."ip_address") FROM "impressions" WHERE "impressions"."impressionable_id" = $1 AND "impressions"."impressionable_type" = $2 [["impressionable_id", 27], ["impressionable_type", "Photo"]]

I guess there should be a DISTINCT thrown in there as well to make it work.

Contributor

Linuus commented Aug 16, 2013

Something seems really off here...

If I run: photo.impressionist_count(:filter => :session_hash) it counts all impressions, although I can see they have the same session_hash.

I get the same result with filter: :ip_address

The SQL generated looks like this:
SELECT COUNT("impressions"."ip_address") FROM "impressions" WHERE "impressions"."impressionable_id" = $1 AND "impressions"."impressionable_type" = $2 [["impressionable_id", 27], ["impressionable_type", "Photo"]]

I guess there should be a DISTINCT thrown in there as well to make it work.

@Linuus

This comment has been minimized.

Show comment
Hide comment
@Linuus

Linuus Aug 16, 2013

Contributor

Impressionist seems to call something like this:

imps.count(:ip_address, distinct: true)

This doesn't seem to work. At least not in Rails 4. How about doing something like this instead:

imps.select(:ip_address).distinct.count

This works in Rails 4 but the .distinct� call does not exist in Rails 3 but instead you'd use .uniq
But the .uniq call seems to raise some issues: rails/rails#7399

So, perhaps just check rails version and keep the current solution if < 4.0 otherwise go with the above?

Contributor

Linuus commented Aug 16, 2013

Impressionist seems to call something like this:

imps.count(:ip_address, distinct: true)

This doesn't seem to work. At least not in Rails 4. How about doing something like this instead:

imps.select(:ip_address).distinct.count

This works in Rails 4 but the .distinct� call does not exist in Rails 3 but instead you'd use .uniq
But the .uniq call seems to raise some issues: rails/rails#7399

So, perhaps just check rails version and keep the current solution if < 4.0 otherwise go with the above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment