Skip to content

Feature/using populate if emtpy to dynamically add songs#5

Open
elvanja wants to merge 9 commits intogogogarrett:masterfrom
elvanja:feature/using_populate_if_emtpy_to_dynamically_add_songs
Open

Feature/using populate if emtpy to dynamically add songs#5
elvanja wants to merge 9 commits intogogogarrett:masterfrom
elvanja:feature/using_populate_if_emtpy_to_dynamically_add_songs

Conversation

@elvanja
Copy link
Contributor

@elvanja elvanja commented May 6, 2014

No description provided.

* introduced album manager for storing albums
* introduced album workflow to handle form submit
* album accepts nested attributes for songs
* using pry gem for debugging
* albums controller uses the album workflow
using latest reform that supports populate_if_emtpy
using latest reform that supports populate_if_emtpy
using latest reform
detecting existing users in album manager
@apotonick
Copy link
Collaborator

Wow, great!!!

Couple of questions:

  1. Do we need all that save code? Isn't that exactly what reform handles now in Form#save?
  2. Can we move the workflow code back to the controller for now? I'm in the process of redefining how that kind of logic is handled in a Trailblazer architecture and I find it more confusing than helping.
  3. When I say "we" I mean "you" 👍

@elvanja
Copy link
Contributor Author

elvanja commented May 16, 2014

Hi Nick,
In given order :-)

  1. I remember that you wanted to have users saved with songs as well. I could not find a nicer solution to "add user if it doesn't exist, but also do not add 2 same users if the same new user is mentioned twice for new songs" problem. If that "user per song" feature is to be dropped, plain form save might work.
  2. Sure thing, I'd still like to do that in my own fork, different branch, since this workflow / data service idea is more in line with @gogogarrett's original idea (I think). When we agree on previous issue, I'll try to make it work.
  3. Sure thing! I guess the same goes when I ask if we can have a new feature in XY gem which you're authoring :-) Glad to be of some help.

By the way, this is the Trailblazer you're talking about? Neat idea! Have you seen http://obvious.retromocha.com/, is it even comparable? Need help with it?

@apotonick
Copy link
Collaborator

So, couldn't we handle 1. with the new :populate option? Regarding 2., that workflow stuff was my original idea, I just made him use it. It doesn't feel too organic anymore.

Obvious looks great, it is kinda like a part of the concepts found in Trailblazer. I can see some points where they could interact. They have a lot in common. However, Trailblazer doesn't only speak about architectural ideas, it gives you all that stuff (e.g. with Reform and Contracts, etc).

I don't know how far the Obvious implementation goes but some parts look almost identical to my concepts, which is fantastic!!!!! Maybe this can save me a lot of work. Thanks man!

@elvanja
Copy link
Contributor Author

elvanja commented May 16, 2014

OK, a short plan: I'll drop "user per song" feature and try to make it all work through form directly. I'll ping you when (not if) I get stuck :-)

Glad you like Obvious!

P.S. Wouldn't mind giving a hand with Trailblazer after this is resolved, even in the form of another "example" project.

@apotonick
Copy link
Collaborator

Nonono, don't drop any features, but leave the code in the controller for now. We can then abstract that into a layer that might become part of Trailblazer. Great that you're helping out, you already helped me so much "just" by using, feedback and contributing to my gems, brother! ❤️

I think the difference between Obvious and Trailblazer is the abstraction level: Obvious is a concept collection pretty similar to what we find in Trailblazer, which is a very concrete framework giving you implementations.

@elvanja
Copy link
Contributor Author

elvanja commented May 19, 2014

:-)

OK, so related to reform example, I've created a new branch: https://github.com/elvanja/reform_example/tree/feature/back_to_controller

Basically, all code from flow and manager moved to controller.
Kept the feature with user per song, and the idea that same new user on the form should be added to the database only once.

Not sure if that was what you wanted, but we can take it from there.

@apotonick
Copy link
Collaborator

Have we considered making use of this: https://github.com/ryanb/nested_form ?

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

Comments