stephenafamo / bob

SQL query builder and ORM/Factory generator for Go with support for PostgreSQL, MySQL and SQLite
https://bob.stephenafamo.com
MIT License
760 stars 39 forks source link

Fix multi sided relationships attachment #212

Closed jacobmolby closed 4 months ago

jacobmolby commented 4 months ago

When having a multisided relationship like this:

 users:
    - name: "users_to_teams_through_team_users"
      sides:
        - from: "users"
          to: "team_user"
          columns:
          - [id,user_id]
        - from: "team_user"
          to: "teams"
          columns:
          - [team_id,id]

With the following tables:

CREATE TABLE `users` (
  `id` int unsigned NOT NULL AUTO_INCREMENT,
  PRIMARY KEY (`id`),
) ENGINE=InnoDB AUTO_INCREMENT=12 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

CREATE TABLE `teams` (
  `id` bigint unsigned NOT NULL AUTO_INCREMENT,
  PRIMARY KEY (`id`),
) ENGINE=InnoDB AUTO_INCREMENT=7 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

CREATE TABLE `team_user` (
  `id` bigint unsigned NOT NULL AUTO_INCREMENT,
  `team_id` int unsigned NOT NULL,
  `user_id` int unsigned NOT NULL,
  PRIMARY KEY (`id`),
  KEY `team_user_team_id_user_id_index` (`team_id`,`user_id`)
) ENGINE=InnoDB AUTO_INCREMENT=7 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci;

It produces the wrong sql for attaching users to teams and vice-versa. Notice that the TeamID is missing, which results in it not being attached properly.

models/teams.go

func attachTeamUsers0(ctx context.Context, exec bob.Executor, count int, teamUsers1 TeamUserSlice, team0 *Team, users2 UserSlice) (TeamUserSlice, error) {
    for i := range teamUsers1 {
        setter := &TeamUserSetter{
            UserID: omit.From(users2[i].ID),
        }

        err := TeamUsers.Update(ctx, exec, setter, teamUsers1[i])
        if err != nil {
            return nil, fmt.Errorf("attachTeamUsers0, update %d: %w", i, err)
        }
    }

    return teamUsers1, nil
}

This change makes sure the team0 variable is used, so the output becomes the following:

func attachTeamUsers0(ctx context.Context, exec bob.Executor, count int, teamUsers1 TeamUserSlice, team0 *Team, users2 UserSlice) (TeamUserSlice, error) {
    for i := range teamUsers1 {
        setter := &TeamUserSetter{
            TeamID: omit.From(team0.ID),
            UserID: omit.From(users2[i].ID),
        }

        err := TeamUsers.Update(ctx, exec, setter, teamUsers1[i])
        if err != nil {
            return nil, fmt.Errorf("attachTeamUsers0, update %d: %w", i, err)
        }
    }

    return teamUsers1, nil
}

I don't know if the changes I made are optimal or (even wrong for some) however for my code base it produced the correct output.

stephenafamo commented 4 months ago

All the checks pass, so I assume it's all good too. Thanks for the contribution

stephenafamo commented 4 months ago

Can you edit a few things?

  1. Make sure that we're not creating more variables than necessary
  2. Possibly rename the variables in the template to be a bit more descriptive.
    As you know, the template can very quickly become unreadable so these things help 🙏🏾
jacobmolby commented 4 months ago

Can you edit a few things?

  1. Make sure that we're not creating more variables than necessary
  2. Possibly rename the variables in the template to be a bit more descriptive. As you know, the template can very quickly become unreadable so these things help 🙏🏾

I tried to make some changes now