voormedia / rails-erd

Generate Entity-Relationship Diagrams for Rails applications
http://voormedia.github.io/rails-erd/
MIT License
4k stars 366 forks source link

cluster tables by explicit domain #320

Open mathieujobin opened 5 years ago

mathieujobin commented 5 years ago

Thank you for implementing #156 in #205

I'd like to be able to go one step further and cluster tables by their domain, without having to namespace them in ruby.

would it be a good idea to add some class method that would do nothing (NoOp) in production. but only communicates this info to the gem in development?

class Requirement
  set_erd_domain :requirement
end
class RequirementCriterion
  set_erd_domain :requirement
end

etc

kerrizor commented 5 years ago

This could be done fairly simply, but freedom patching ActiveRecord::Base to add the DSL, and store the erd domain in a class variable. I believe the code for creating domain clusters would look nearly identical to the code in #205

mathieujobin commented 5 years ago

DSL methods could be

set_erd_cluster_name :foo
set_erd_cluster_color :purple
jsgarvin commented 5 years ago

Is anybody working on this? If not, I may take a stab. I found this issue while looking specifically for a way to create arbitrary clusters.

It feels a bit weird to me putting this right into the AR models though, as it feels beyond the scope of their responsibility. I was imagining something along the lines of a new option in the .erdconfig that takes a hash of keys as the cluster names, and an array of values for each representing the models in that cluster.

Seeing that the feature wasn't currently available, I threw together this rough patch that works well enough in my project...

module RailsERD
  class Domain
    class Entity
      alias_method :namespace_without_patch, :namespace

      def namespace
        clusters = RailsERD.options[:clusters]
        clusters.keys.each do |key|
          return key if clusters[key].include?(name)
        end
        namespace_without_patch
      end
    end
  end
end

If nobody's working on this, and the above approach seems acceptable, I'd be happy to try to put together a more graceful implementation in a PR.

Maybe, instead of just an array of model names, could also include a hash of other cluster options (eg. clusters: { Foo: { models: ['Bar', 'Baz'], options: { bgcolor: 'purple', style: 'dashed' }}, ...}

mathieujobin commented 5 years ago

I love it,... keeping this all in one place is very good for documentation as well. you are right it does not need to be in the models...

I will happily test drive your patch with my project once ready

jsgarvin commented 5 years ago

Ok, having given this some more thought, I have another idea that I think I like even better. This will allow not only placing models/nodes into arbitrary clusters, but set (just about?) any Graphiz supported attribute on individual nodes or clusters. This would add two new options to .erdconfig, that would look something like...

clusters:
  Infrastructure:
    attributes:
      style: dotted
      bgcolor: purple
  Furniture
    attributes:
      style: dashed
      bgcolor: green
models:
  Building:
    cluster_name: Infrastructure
    node_attributes:
      style: filled
      fillcolor: blue
 Room:
   cluster_name: Infrastructure
   node_attributes:
     fontsize: 20
  Table:
    cluster_name: Furniture
    ...etc

I have a rudimentary but functional POC working in my env. I think I would just need to clean it up and add some tests.

I think I would probably do this as two separate PRs. The first just to add the new config options to pass attributes to nodes and clusters, and then a second that builds on top of the first to be able to assign nodes to arbitrary clusters.

mathieujobin commented 5 years ago

Wouldn't it be easier to write having models under each clusters?

mathieujobin commented 5 years ago

Would STI subclasses automatically be picked up? Or we should defined them as well?

jsgarvin commented 5 years ago

Good question. Looking up the additional node attributes for models when needed would be more awkward if they were buried under some cluster name somewhere. At that point in the current code, it doesn't know, or care, about the cluster. It would be much more straight forward to find the node attributes if the models were at the top level. The only reason for the clusters section here is to be able to define additional attributes (eg, color) to the clusters. Plus, I think one should be able to set additional attributes on the model nodes without having to put them into clusters. This way, users would have that option as well.

As for STI subclasses, I think those would need/get to be defined individually, but that's just an educated guess at this point.

jsgarvin commented 5 years ago

Well, can't seem to get tests to run locally with a clean checkout. Specified 'sqlite3' for database adapter, but the gem is not loaded. Add "gem 'sqlite3'" to your Gemfile (and ensure its version is at the minimum required by ActiveRecord) But, it's there in the Gemfile, and most tests appear to be passing on Travis (but a lot are not). So....

jsgarvin commented 5 years ago

Well, here's a couple branches without tests that are working for me. https://github.com/jsgarvin/rails-erd/tree/add_ability_to_set_node_parameters and https://github.com/jsgarvin/rails-erd/tree/add_support_for_arbitrary_clustering . Second builds on top of the first. Don't want to open PRs with supporting tests, but, until I can get the tests to run with a fresh checkout... :wink:

mathieujobin commented 5 years ago

@jsgarvin would it be hard to have the option to print each domain/cluster on a different page?

mathieujobin commented 5 years ago

a couple months later to the party ! finally I am able to try this with my large project

165 tables, 197 models makes it a lot easier to generate something meaningful.

for splitting into multiple pages, I found pdfposter

I will use @jsgarvin fork until this get merged

thank you

Th0t commented 4 years ago

@jsgarvin > https://github.com/jsgarvin/rails-erd/tree/add_support_for_arbitrary_clustering that feature is a must have ! I'm really enjoying it. Is there a chance to see that on the official repo ?

hooopo commented 4 years ago

@mathieujobin I think you can use https://github.com/drawerd/drawerd , it depentents on rails-erd, and you can set group and filter svg graph for that.

mjobin-mdsol commented 2 years ago

I rebased @jsgarvin 's work in my fork with current master https://github.com/mjobin-mdsol/rails-erd/tree/jsgarvin-cluster