Skip to content

Conversation

@sauloperez
Copy link
Collaborator

@sauloperez sauloperez commented Jan 30, 2018

This fixes the default order of the users list by the member_uid.

ActiveRecord doesn't notice that we need to join with members in two parts of the ActiveRecord::Relation that we're building and it joins it two times adding an alias called members_users, instead of joining that table just once.

As the constraint on the organization_id was referencing the members_users table this wasn't picked up by Postgres while joining the tables. Specifically, when joining users with members, thus returning all members of a user, regardless of their organization.

So far, the query that gets executed from the users controlle is (See https://github.com/coopdevs/timeoverflow/blob/develop/app/controllers/users_controller.rb#L7):

SELECT "users"."id" AS t0_r0, "users"."username" AS t0_r1, "users"."email" AS t0_r2, "users"."password_digest" AS
 t0_r3, "users"."date_of_birth" AS t0_r4, "users"."identity_document" AS t0_r5, "users"."phone" AS t0_r6, "users"."alt_phone" AS t0_r7, "use
rs"."address" AS t0_r8, "users"."created_at" AS t0_r9, "users"."updated_at" AS t0_r10, "users"."deleted_at" AS t0_r11, "users"."gender" AS t
0_r12, "users"."description" AS t0_r13, "users"."active" AS t0_r14, "users"."terms_accepted_at" AS t0_r15, "users"."encrypted_password" AS t
0_r16, "users"."reset_password_token" AS t0_r17, "users"."reset_password_sent_at" AS t0_r18, "users"."remember_created_at" AS t0_r19, "users
"."sign_in_count" AS t0_r20, "users"."current_sign_in_at" AS t0_r21, "users"."last_sign_in_at" AS t0_r22, "users"."current_sign_in_ip" AS t0
_r23, "users"."last_sign_in_ip" AS t0_r24, "users"."confirmation_token" AS t0_r25, "users"."confirmed_at" AS t0_r26, "users"."confirmation_s
ent_at" AS t0_r27, "users"."unconfirmed_email" AS t0_r28, "users"."failed_attempts" AS t0_r29, "users"."unlock_token" AS t0_r30, "users"."lo
cked_at" AS t0_r31, "users"."locale" AS t0_r32, "users"."notifications" AS t0_r33, "members_users"."id" AS t1_r0, "members_users"."user_id"
AS t1_r1, "members_users"."organization_id" AS t1_r2, "members_users"."manager" AS t1_r3, "members_users"."created_at" AS t1_r4, "members_us
ers"."updated_at" AS t1_r5, "members_users"."entry_date" AS t1_r6, "members_users"."member_uid" AS t1_r7, "members_users"."active" AS t1_r8,
 "accounts"."id" AS t2_r0, "accounts"."accountable_id" AS t2_r1, "accounts"."accountable_type" AS t2_r2, "accounts"."balance" AS t2_r3, "acc
ounts"."max_allowed_balance" AS t2_r4, "accounts"."min_allowed_balance" AS t2_r5, "accounts"."flagged" AS t2_r6, "accounts"."created_at" AS
t2_r7, "accounts"."updated_at" AS t2_r8, "accounts"."organization_id" AS t2_r9
FROM "users"
INNER JOIN "members" "members_users"
  ON "members_users"."user_id" = "users"."id"
INNER JOIN "accounts"
  ON "accounts"."accountable_id" = "members_users"."id"
  AND "accounts"."accountable_type" = 'Member'
INNER JOIN "members"
  ON "users"."id" = "members"."user_id"
WHERE "members"."organization_id" = 1
ORDER BY "members_users"."member_uid" ASC;

With this changes it becomes:

SELECT (same columns selected as above)
FROM "users"
INNER JOIN "members" "members_users"
  ON "members_users"."user_id" = "users"."id"
INNER JOIN "accounts"
  ON "accounts"."accountable_id" = "members_users"."id"
  AND "accounts"."accountable_type" = 'Member'
WHERE "members"."organization_id" = 1
ORDER BY "members_users"."member_uid" ASC

ActiveRecord doesn't notice that we need to join with `members` in two
parts of the ActiveRecord::Relation and it joins it two times adding
an alias called `members_users`, instead of joining that table just once.

As the constraint on the `organization_id` was referencing the
`members_users` table this wasn't picked up by Postgres while joining
the tables. Specifically, when joining `users` with `members`, thus
returning all members of a user, regardless of their organization.
@sauloperez sauloperez force-pushed the fix/users-list-sorting branch from a63f04a to 2d3bb1c Compare January 30, 2018 16:19
@sauloperez sauloperez merged commit 6660a79 into master Jan 30, 2018
@sauloperez sauloperez deleted the fix/users-list-sorting branch January 30, 2018 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants