preaction / Yancy

The Best Web Framework Deserves the Best Content Management System
http://preaction.me/yancy/
Other
54 stars 21 forks source link

Allow collection configuration from with DBIx::Class result definition #37

Closed mario-minati closed 5 years ago

mario-minati commented 5 years ago

Add distributed collection configuration for dbi backend through a custom method in each result definition. The idea is to allow all openapi database related configuration to happen in one place.

mario-minati commented 5 years ago

A DBIx::Class::Yancy could look like the following:

package DBIx::Class::Yancy;
use strict;
use warnings;
use base qw( DBIx::Class );

use namespace::clean;

=head1 NAME

DBIx::Class::Yancy - Set Yancy collection config in result class.

=head1 SYNOPSIS

In your Schema or DB class add "Yancy" to the component list.

  __PACKAGE__->load_components(qw( ... Yancy ... ));

Specify the yancy collection config for the table.

  package My::Item;
  __PACKAGE__->yancy({
    read_schema => 1,
    title => "Fany item table",
    'x-id-field' => 'email',
    'x-list-columns' => [
        { title => "Person", template => '{name} <{email}>' },
    ],
    api_controller => 'My::Fancy::Yancy::API::Controller',
  });

If you don't, specify a custom config the table will be ignored.
The default config is like:

  package My::Item;
  __PACKAGE__->yancy({
    'x-ignore' => 1,
  });

=cut

__PACKAGE__->mk_classdata( 'yancy' => {'x-ignore' => 1} );

1;
mario-minati commented 5 years ago

As we decided to move the yancy collection config into our mojolicious config for more flexibility, we will not go on with this.

preaction commented 5 years ago

Having that component set the default to ignore is pretty useful I think. This would be a great CPAN distribution (but not part of the core Yancy to prevent the dependency).

If you'd like to do that, it sounds like a good idea to me, and I can help with the process of getting a CPAN distribution together and uploaded. Otherwise, with your permission I could write some tests and upload this to CPAN myself (crediting you, of course).

mario-minati commented 5 years ago

Sure, we can put in on CPAN. In this case we write some more docs to reference your docs. Regarding tests we would appreciate your help. We will setup a github repo, so we can share the development work. :-)

In this case you extend the Yancy Dbi part to test if the result class can('yancy'), right?

preaction commented 5 years ago

Right, Yancy::Backend::Dbic sub read_schema would expect a yancy sub on the ResultSource object (which it'd look for using can('yancy')).

But, you wouldn't need to set read_schema => 1 inside the DBIx::Class class, since read_schema => 1 is the only way this information would be looked up in the first place.

mario-minati commented 5 years ago

Right, I will change that.

mario-minati commented 5 years ago

Initial upload in new repo for DBIx::Class::Yancy is done. I'll add DBIx::Class related tests tomorrow.

mario-minati commented 5 years ago

Intial version state is reached (v0.001) and is availabe on Github, but we didn't upload to CPAN as the tests should be tested against the next Yancy version that supports DBIx::Class::Yancy and maybe extended with tests for the resulting collection config.

preaction commented 5 years ago

Okay, sorry, so this is waiting on me to add support to the backend. I'll get that done and update you when I've done so.

mohawk2 commented 5 years ago

Do I understand right that $dbic_source->yancy would be called by read_schema, and return a snippet of JSON schema that would be the starting point of the schema for that collection? (especially with regards to x-ignore)

If so, that should only be a few lines of code. In the test-suite's DBIC classes, we could test by adding that to mojo_migrations, which would be nicely idiomatic.

mohawk2 commented 5 years ago

It was two lines of code, and allowed the deleting of a special case in the testing! Addressed in #59.