The `diesel::Identifiable` trait is not derived for the table a foreign key refers to

[longsleep]

In https://github.com/Wulf/dsync/blob/a44afdd08f4447e367aa47ecb91fae88b57f8944/src/code.rs#L214 the Identifiable traits gets derived for any table which has a foreign key. That is fine and all but forgets the table the foreign key comes from.

A super simple fix goes like this

diff --git a/src/code.rs b/src/code.rs
index e982a36..277a248 100644
--- a/src/code.rs
+++ b/src/code.rs
@@ -198,6 +198,8 @@ impl<'a> Struct<'a> {

                 if !self.table.foreign_keys.is_empty() {
                     derives_vec.extend_from_slice(&[derives::ASSOCIATIONS, derives::IDENTIFIABLE]);
+                } else if !self.table.primary_key_columns.is_empty() {
+                    derives_vec.push(derives::IDENTIFIABLE);
                 }
             }

When a table has a primary key that usually should mean that Identifiable should work. Dsync already generates the correct other fields related to this trait at other places. See https://docs.diesel.rs/2.0.x/diesel_derives/derive.Identifiable.html.

The above patch works for me. This issue can be used for discussion in case I missed something, before I make a PR for this.

[hasezoey]

the Identifiable traits gets derived for any table which has a foreign key. That is fine and all but forgets the table the foreign key comes from.

i dont fully understand what it is forgetting; do you mean "forgetting" as in forgetting to add Identifiable on the foreign struct itself?

[longsleep]

i dont fully understand what it is forgetting; do you mean "forgetting" as in forgetting to add Identifiable on the foreign struct itself?

Yes. #135 fixes the issue. Users of those structs might expect them to be Identifiable and make use of this trait in own code.

[Wulf]

hey @longsleep, not sure if you're still working with rust (I see some Go in your recent activity), but I wanted to ask for your help in maintaining this repo. Could you join @hasezoey as a maintainer?

Also @hasezoey I saw your name pop up in some steam forums haha. I was really delighted to see and recognize a familiar avatar :hugs: . Thanks for looking out for PRs and merging them in. I saw your comment regarding branch protection rules. Do you want me to remove those?

[hasezoey]

I saw your comment regarding branch protection rules. Do you want me to remove those?

i mean i understand and think they are OK rules, though they only really work if there is another active pair of eyes that can approve (and has write-access)

[longsleep]

hey @longsleep, not sure if you're still working with rust (I see some Go in your recent activity), but I wanted to ask for your help in maintaining this repo. Could you join @hasezoey as a maintainer?

I don't think that I am exactly the right guy as my use case is just a fraction of the project's scope described in the README (For example I have no clue about create-rust-app) means only familiar with a fraction of the code/features in dsync.

I already maintain a lot of things (mostly non-public though, see https://gitlab.com/longsleep) my time is limited. It might still make sense to add me as a maintainer if it helps if I would share my opinion every once in a while (like to get things committed because of merge rules). If this works good enough depends on the volume of changes and the frequency.

[longsleep]

Closing this since it was fixed via #135

[Wulf]

(For example I have no clue about create-rust-app)

@longsleep -- if it's just that, then I'm not worried :) dsync was initially built for create-rust-app, but it's actually for diesel codegen in general. I'm hoping we keep the codebase simple / easy for new contributors to jump into and help develop.

volume of changes and the frequency

I remember both @hasezoey and I both believed this crate reached a stable state. I'm still hoping to experiment with advanced queries (for relationships, for example) but I don't think that will be anytime soon.

I'll add you to the maintainers list! Let's move forward with the review rules intact and revisit this if it's still an issue.