Namespacing spree so that it can integrate with RefineryCMS, other apps easily

[Josh Adams]

So has any work been done with spree to namespace it?  RefineryCMS's master (aimed at rails 3.1), which will inevitably be released as a version of some sorts in the nearish future, has fully namespaced everything under the Refinery:: namespace, so all models/tables look like ::Refinery::User, refinery_user now.  This makes using it with other apps fairly workable.

Spree currently doesn't seem to have any namespacing, but I was told that it supports being a mounted app these days.  I just wanted to start a conversation around all of this.

As a bit of background, I stepped into refinerycms contributorship relatively recently, but did a lot of the rails 3 -> rails 3.1 work because I needed to get it and forem integrating nicely.  I've taken care of this part (forem mounts into my refinery app nicely now) on master of both projects (I contribute to forem now as well).  I've got a great deal of interest in seeing that the Rails ecosystem generally starts plugging in together nicely, so just trying to talk about this sort of thing wherever I can.

Thoughts / information on this?

Thanks very much,

Josh Adams
Isotope11

[Brian Quinn]

Hi Josh,

I haven't focused on name-spacing as part of the Rails 3.1 upgrade, it's a pretty invasive change (as it would break pretty much every extension).

We'd need to get some consensus from the community if they would be happy with such a change. My personal view is it's a good idea, and actually pretty easy to implement in core, but it would require a lot of updates in Spree applications and their extensions.

Maybe the best solution is to have it as an optional setting, but I don't know how achievable that might be.

Brian Quinn

-------------------------------------------

Rails Dog LLC

2 Wisconsin Circle, Suite 700

Chevy Chase, MD 20815

voice: (301)560-2000

-------------------------------------------

--
You received this message because you are subscribed to the Google Groups "Spree" group.
To view this discussion on the web visit https://groups.google.com/d/msg/spree-user/-/XpJq8Pv9tRgJ.
To post to this group, send email to spree...@googlegroups.com.
To unsubscribe from this group, send email to spree-user+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/spree-user?hl=en.

[Josh Adams]

So yeah, I don't think it could work as an optional setting.  RefineryCMS had to bite the bullet and just do it, and make another biggish release that includes it (upcoming).  It does require a bit of work on all extensions to support the new hotness, but it also makes it drastically easier to integrate the app with other apps.  I'm of the opinion that any app like refinery or spree should be doing this, and very grateful that they opted to take care of it sooner rather than later.

Until the rails ecosystem is used to doing this sort of thing, we will always have problems integrating apps like these.  Just my $0.02.

-Josh

[Sean Schofield]

Josh,

I'm really interested in making the changes to support a Spree
namespace. I agree with Brian that it will invariably break a lot of
things but I agree with you that the benefits outweigh the problems.

The potential for being able to mount multiple apps together (ex.
Refinery + Spree) is just too compelling to ignore. Sure you can add
Spree to an existing Rails app but you run the risk of colliding with
namespaces, etc.

It would be so cool to add a Spree store to an existing Rails site
simply by doing the following:

Foo::Application.routes.draw do
mount Spree::Engine => "/store"
end

I think if we're going to contemplate doing this (and that's all we're
doing for now .. contemplating it) then we should do it before Spree
is 1.0.

@Josh: Are you volunteering to help with some of the heavy lifting?
If so, that would certainly increase my willing to consider it.

What do other people think on this topic? For reference you may want
to see the excellent Ryan Bates screen cast on Engines which discusses
the namespace stuff in some detail[1].

Sean

[1] http://railscasts.com/episodes/277-mountable-engines

> --
> You received this message because you are subscribed to the Google Groups
> "Spree" group.
> To view this discussion on the web visit

> https://groups.google.com/d/msg/spree-user/-/KOOV9FBJ0KAJ.

[Christopher Maujean]

>
> Foo::Application.routes.draw do
>  mount Spree::Engine => "/store"
> end
>

I think this is excellent, and exactly where spree and refinery need
to be headed. I would be happy to put some time in on this.

--Christopher

[josh jacobs]

I've only been working with Spree for about two months, but I am planning on adopting it pretty heavily for some new project. I would volunteer to help with a task like this as well.

[Josh Adams]

Sean,

I would be glad to get involved a little bit.  For what it's worth, forem is there (you can mount a forum on your app in exactly the way you described).  Refinery acts nicely with other apps, but is not yet explicitly mountable.  I can't promise too very much time because I'm constantly busy, but after September I'm stepping away from most of my billable work to work more within my company and to further some open source efforts I have a vested interest in, and this one would qualify.

The first step would definitely be to namespace everything.  As for future steps, I'm not exactly sure.  I'm going to ping Ryan Bigg on this as well because he's seemingly interested in this sort of thing within Rails, to see if I can at least get some direction from him so we Do Things Right.

I guess my point is: I'm in, but I don't know how much focus I'll be able to give it in the short term.  Let's just get a branch out there and start hacking on it.  We'll need to make sure that the branch we're doing this on keeps merged with master as much as possible (there will be massive merge conflicts) to avoid it dying a death later.  I'll also ping Phillip Arndt to see if he can give us pointers on that front, because he handled it swimmingly with Refinery.

I'm getting involved with all sorts of projects these days :)

-Josh

[Ryan Bigg]

Hello,

I received Josh Adams' ping and I was compelled to give my thoughts here too.

Josh is correct in stating that with forem you simply do:

mount Forem::Engine => "/forem"

in the application and works, to a degree. There's additional configuration required such as telling Forem what the "User" class of the application is and what not. We could not find a way to avoid that.

If you namespace everything as Josh suggests, you will inevitably piss off some people. Haters gonna hate, basically. But the benefit is that in applications that are using Spree, they may have their own Product model and still have access to Spree's at Spree::Product rather than it's current location. This namespacing will be a large undertaking and it's of course not just the models, you'll need to do the controllers, helpers and views too, as well as anything else that is "Spree-specific".

The benefits *far* outweigh the consequences here. Namespacing is the way of the future and something I believe Spree should undertake as part of their 1.0 release. After all, what is a major release without a large breaking change? :)

[Brian Quinn]

I'm just going to throw this idea out there and see what people think!

Let us for a minute assume that namespacing has happened for a couple of great apps (like Refinery, Forem, Spree, etc), and that user / authentication can be shared between all the engines. So now you can create an application and mount all three - suddenly you've got kick-ass cms, forum and e-commerce functionality all living inside one application. 

BUT you've got three different administration systems that: all look different and all need to be accessed from different URLs

Should we try to create an shared administration framework that each application can plug it's necessary admin features into? So as you install engines the shared administration system gets new tabs / menu options / whatever? 

I know there's apps like http://activeadmin.info/ out there, but I'm personally not a fan of that approach as it enforces to many dependencies / choices. I'm suggesting we keep the framework as basic as possible, and not enforce too many choices on any project that may consider using it. So each application can provide their own controllers, use whatever libraries they want. It's just the view part of the admins that we tie together?

Bad idea?

Brian Quinn

-------------------------------------------

Rails Dog LLC

2 Wisconsin Circle, Suite 700

Chevy Chase, MD 20815

voice: (301)560-2000

-------------------------------------------

--
You received this message because you are subscribed to the Google Groups "Spree" group.

To view this discussion on the web visit https://groups.google.com/d/msg/spree-user/-/SD3cvT464TwJ.

[Sean Schofield]

Shared admin is something that Yehuda dreamed about a couple of years
ago when we started down the mountable apps route. I think we should
move to namespaced mountable app first before we worry about it. We
could probably pitch in with these other projects at some point down
the line to solve it.

Sean

[Sean Schofield]

Ryan - Thanks for weighing in. I agree with your assessment.

@Volunteers: Thanks for offering to help. We're definitely going to
take you up on it. I think we should plan on taking this up about two
weeks from now when we've got a stable Rails 3.1 compatible release
out the door. We've also got some pull requests and bug reports that
have been neglected while we finished up the Rails 3.1 merge.

Sean

> --
> You received this message because you are subscribed to the Google Groups
> "Spree" group.
> To view this discussion on the web visit

> https://groups.google.com/d/msg/spree-user/-/SD3cvT464TwJ.

[Josh Adams]

Glad to hear this effort is going to be embarked upon :)  I dream of the day when the rails ecosystem all slots together nicely, and we're rapidly approaching it.

-Josh

[liebana]

Hi,

Any news about this? How can we help?

I'm a little bit late into the conversation, but as Sean replied
before, IMHO the focus should be in namespacing features before
thinking about the shared administration app, that I think it's
optional, as far as in each admin bakoffice there are links to jump
from one to another safely.

We're dreaming about integrating Spree and Refinery.

Best regards.

[Sean Schofield]

We're definitely still planning on doing this. Right now we're
talking with different people about the best way to have Spree
integrate with any Rails app (not just Refinery.) So we're trying to
explore different options for this before we jump in and start coding.

I agree that namespacing is a necessary first step and that will begin
in the very near future. We're working on getting Spree 0.70.0 out
the door in the next few days and then we'll turn our attention to
this. I plan on coming back to this thread and contacting each of the
people on this thread individually so we can coordinate a joint
effort.

Stay tuned.

Sean

> --
> You received this message because you are subscribed to the Google Groups "Spree" group.

[Sean Schofield]

The process has now begun. We just created a branch for this purpose:

https://github.com/spree/spree/tree/namespace

Will do a blog post on this shortly but in the meantime, please feel
free to submit pull requests and feedback.

Sean

[Ryan Bigg]

Howdy, 

I'm new here. Excuse my ignorance / buffoonery. 

I'm also a writer, so I write a lot of words. Most of it is useful. There may even be some jokes.

I've taken on the responsibility of namespacing after discussing with Sean, but I could still use some help. So far, I've done just the controllers and the files within the lib directory, but there's still much more work to be done.

Let me explain why we should be namespacing first before I dive into what needs to be done. The best example is probably the controllers. If I have a ProductsController in my application right now and I want to use Spree, Spree's ProductsController is going to be overriden by my application's ProductsController. To get Spree's functionality into the application properly, I would need to a) rename my ProductsController to something else or b) copy over Spree's ProductsController's code into my application's ProductsController. 

This is not ideal. If Spree issues an update for the ProductsController, then I would need to go through the process of updating that code again.

So with the namespacing what would happen (actually, what *has happened*) is that Spree has a Spree::ProductsController now. Now, this is completely separate from the ProductsController of the application, much in the same way that Ryan Bates and I (Ryan Bigg) are obviously two separate people based on the "namespacing" of our last names.

Now, this opens up other opportunities also. Imagine you don't want to have your own ProductsController in your application and you're *mostly* happy with the one that Spree provides but you really don't like one thing that it's doing. To fix this, you would define a new controller in your application at app/controllers/products_controller.rb or perhaps app/controllers/spree_overrides/products_controller.rb like this:

    class ProductsController < Spree::ProductsController

      def index

        # some different functionality

      end

    end

Then, to get Spree to use *this* controller rather than the original, you would customise it in the routes. At least, this is how I think it would work. The line would look like this:

    spree :controllers => { :products => "products" }

By default, the controller here would be "spree/products", but by specifying this option in the routes you would be telling Spree, "well actually, it's over here" and Spree would dutifully look there. Due to you inheriting from `Spree::ProductsController`, it would use the old views as well. You can override these as you wish.

--------

Alright, now that we've covered the *why*, let's look at the *how*:

First: I'm kind of waving the scalpel around in the dark at the moment as there aren't that many tests for the functionality provided by the core engine. i.e., if I happened to accidentally delete all the actions in, say, Admin::AdjustmentsController, and then run the tests, there's *nothing* there that tells me "hey idiot, you made a mistake".

So, we need tests. Lots of them. Both the controller and integration kind. Currently, Spree uses Cucumber for the integration tests, although I would probably push for straight RSpec+Capybara for two reasons: 1) we can run a single command to run all the tests and 2) I think Cucumber is an unnecessary layer between the coder and the API in a project like Spree which is used by people who (I think) want to drop it into their project and have it work.

The tests will also probably save us from regressions by bugs, although I think the process at the moment  has you submit a regression test for the bug when you find one, which is really awesome :)

Second: Helpers. These need to be namespaced, and that's easy enough done. We need tests again to make sure that they are still working. (Sorry, testing fanatic.)

Third: Models. Spree's got a lot of models. 

I think I once saw a kitchen_sink.rb in one of the extensions, but I can't remember which one and I can't be bothered looking right now.

These will need to be namespaced within the Spree module also, and the tables for these models will also need to be renamed to spree_[table_name]. This change would allow the application to have its own `Product` model if it wished and a products table, and Spree wouldn't care.

Fourth: Mailers. Same deal as controllers + models. Relatively easy, but we will need to change references to them in the engine to point to their new places.

Fifth: Further namespacing of things in lib. There's currently things in lib that are defined as top-level constants, which I think should be namespaced purely because everything else is namespaced too. No point being different in one little tiny area just because that's the way it was in the past. Consistency rules!

-----

*catches his breath*

After all that is done, I think we'll have a fully-namespaced engine (just core) that won't clash with applications or other (properly-namespaced) engines.

I hope this gives you some idea of the scope that is involved and work that needs to be done in order to get there.

Thanks!

Ryan Bigg

[Jones Lee]

@Ryan: You are a great writer with touch of humor :). Firstly, thanks for your work on namespacing, it is soon we would be able to integrate nicely with other Rails app.

Regarding the integration test method, imho i think we should keep Cucumber for the sake of readability. I understand that Spree users are mainly developers, yet having a readable form would help newcomers pick up the code faster.

Your TODO list is long. I am just wondering if you could break up the tasks into small tasks and post them up onto GH Issues so developers. It would help avoid redundancy. Just my 2cent.

[Brian Quinn]

Great stuff Ryan, I think you've covered the namespacing challenge nicely. 

I've been thinking about replacing cucumber with rspec request specs for a while, I've ever written a few and just not committed them yet. So that gets a massive plus one from me! I volunteer to start the conversion process once we get a little more acceptance? (schof). 

Jeff Squires I know will be sure to help with the rspec conversion too, right? ;)

[Jeff Squires]

Oh no! I've been called out! :-)

Without getting religious about it...I can write request specs faster
than cucumber tests...so...

I'm happy to help out either way, but I prefer request specs, with no
eloquent explanation of why.

Ryan, thanks for the detailed writeup. I thought I knew why we needed
namespacing, now i understand it.

On Oct 6, 7:04 am, Brian Quinn <br...@railsdog.com> wrote:
> Great stuff Ryan, I think you've covered the namespacing challenge nicely.
>
> I've been thinking about replacing cucumber with rspec request specs for a while, I've ever written a few and just not committed them yet. So that gets a massive plus one from me! I volunteer to start the conversion process once we get a little more acceptance? (schof).
>
> Jeff Squires I know will be sure to help with the rspec conversion too, right? ;)
>

> >> On Tue, Sep 6, 2011 at 12:27 PM, Sean Schofield <s...@railsdog.com> wrote:
> >>> We're definitely still planning on doing this.  Right now we're
> >>> talking with different people about the best way to have Spree
> >>> integrate with any Rails app (not just Refinery.)  So we're trying to
> >>> explore different options for this before we jump in and start coding.
>
> >>> I agree that namespacing is a necessary first step and that will begin
> >>> in the very near future.  We're working on getting Spree 0.70.0 out
> >>> the door in the next few days and then we'll turn our attention to
> >>> this.  I plan on coming back to this thread and contacting each of the
> >>> people on this thread individually so we can coordinate a joint
> >>> effort.
>
> >>> Stay tuned.
>
> >>> Sean
>

> >>> On Tue, Sep 6, 2011 at 12:13 PM, liebana <carloslieb...@gmail.com> wrote:
> >>>> Hi,
>
> >>>> Any news about this? How can we help?
>
> >>>> I'm a little bit late into the conversation, but as Sean replied
> >>>> before, IMHO the focus should be in namespacing features before
> >>>> thinking about the shared administration app, that I think it's
> >>>> optional, as far as in each admin bakoffice there are links to jump
> >>>> from one to another safely.
>
> >>>> We're dreaming about integrating Spree and Refinery.
>
> >>>> Best regards.
>
> >>>> On 5 ago, 15:11, Josh Adams <josh.ruby...@gmail.com> wrote:
> >>>>> Glad to hear this effort is going to be embarked upon :)  I dream of the day
> >>>>> when the rails ecosystem all slots together nicely, and we're rapidly
> >>>>> approaching it.
>
> >>>>> -Josh
>
> >>>> --
> >>>> You received this message because you are subscribed to the Google Groups "Spree" group.
> >>>> To post to this group, send email to spree...@googlegroups.com.
> >>>> To unsubscribe from this group, send email to spree-user+...@googlegroups.com.

> >>>> For more options, visit this group athttp://groups.google.com/group/spree-user?hl=en.

>
> >> --
> >> You received this message because you are subscribed to the Google Groups "Spree" group.
> >> To post to this group, send email to spree...@googlegroups.com.
> >> To unsubscribe from this group, send email to spree-user+...@googlegroups.com.

> >> For more options, visit this group athttp://groups.google.com/group/spree-user?hl=en.

>
> > --
> > You received this message because you are subscribed to the Google Groups "Spree" group.
> > To post to this group, send email to spree...@googlegroups.com.
> > To unsubscribe from this group, send email to spree-user+...@googlegroups.com.

> > For more options, visit this group athttp://groups.google.com/group/spree-user?hl=en.

[Sean Schofield]

Super excited about the work on the name space stuff. Thanks for
taking the initiative on this Ryan. Hopefully the other volunteers
from earlier on this thread are still interested in helping.

JD is going to take a crack at porting over some of the models. I'm
recommending people just do small chunks at a time and issue pull
requests (use GH label for 'namespace'.) This way we won't be
duplicating work. For now I think we can pick random models, helpers,
etc. and start porting them over.

Regarding the tests, I'm in favor of more RSpec (and more tests in
general.) I don't think we should abandon the Cucumber stuff, a lot
of work went into that and I think its helpful to have some basic
browser coverage that exercises the javascript, etc. That said, if
you need to write a new test for something I don't think we need to do
it in cucumber.

So lets not throw cucumber away. In fact, I think maybe we
consolidate it into a new top-level features directory where we can
run all of the cukes (and see all of the steps, etc.) all in one
place.

Will try to get to a few of the models myself tonight to do my part.
I've been a little preoccupied lately but its going to be obvious what
I've been up to shortly when we make our next announcement.

Sean

[John Dyer]

Hey Ryan,

I've begun working on namespacing all the top level models. Essentially just renaming the models and their respective tables and moving them into app/models/spree. Hope this helps accomplish what we want to achieve here. I will be pushing my branch at the end of the day so you can take a look at it.

Cheers,

John Dyer

[Sean Schofield]

FYI - I told John to go ahead with this effort even though its going
to break some things. There's really no way to keep everything
functional without fixing everything all in one shot. Since this is
on its own branch I think its fine to go full speed ahead and get
everything back to working condition as quickly as possible.

Sean

> --
> You received this message because you are subscribed to the Google Groups
> "Spree" group.

> To view this discussion on the web visit

> https://groups.google.com/d/msg/spree-user/-/54Jv9jU5sFoJ.

[Ryan Bigg]

Thanks John for getting started on the model namespacing!

I took some time today to get the tests passing again on the namespace branch, and now I'm back to having the two tricky controller specs failing. 

The next thing to do now is to namespace all the stuff in the lib directory correctly (except generators, leave that where it is) and then I reckon we're done? Wow :)

[Ryan Bigg]

Turns out the controller specs weren't tricky at all. Fixed + pushed.

[Josh Adams]

So I was certainly one of the guys that volunteered for this initially.  I'm still very interested.  I cannot jump in and help presently as I've been in the hospital for a week now and am not sure when I'm getting out (nothing terminal, just a lot of pain and the treatment isn't very speedy).  Just wanted to make sure I pinged you, Sean, so you wouldn't think I was some abandoner-guy :)

Anyway, I'll get involved when I can.  Love how quickly this is going.

-Josh

[Peter Berkenbosch]

Get well soon Josh!!!

Peter Berkenbosch

--------—-------—---------—

Wat niet kan, is nog nooit gebeurd.

-------—--------—-----------—

Verstuurd vanaf mijn iPhone.

[Sean Schofield]

Josh,

There will always be plenty of ways to help on the Spree project when
you come back. Wishing you a speedy recovery.

Sean

[Ryan Bigg]

I didn't really want to start a new thread for this, so I thought I'd add it here.

This weekend and as late as today, I've seen reports of a "wrong argument type Module (expected Class)" in the namespace branch, happening when you try to run "bundle exec rspec" or "bundle exec cucumber" inside the core directory.

I had fixed it on the weekend (so I had thought) and then it re-appeared when Jones Lee was doing testing yesterday. I said I would take a look at it last night but was too tired.

Tonight's a different story. I have more energy than a full kindergarten and an extraordinary amount of patience with code. I sat down with it today and went through it and eventually solved it.

Previously, we had this:

class Spree::Admin::AdjustmentsController < Spree::Admin::ResourceController

Which is perfect and lovely and fine. But personal-love for the more extended version took over, so I converted it into this:

module Spree

  module Admin

     class AdjustmentsController < ResourceController

       # code goes here

     end

   end

end

A little more beautiful, I think. Less repetition of that nasty Spree::Admin part. And it worked. I promise!

Then suddenly, it didn't. The error reports look like this:

core/app/controllers/spree/admin/adjustments_controller.rb:4:in `<module:Admin>': wrong argument type Module (expected Class) (TypeError)

Which had me thinking that it was to do with the Admin module, mainly because I was trying out for Village Idiot and was considered a front-runner for a time. But if you *look* at the error message, it says the error is INSIDE the Admin module. So what's inside that?

     class AdjustmentsController < ResourceController

See where this is going? No? I didn't either. I learned things about Ruby I didn't even know.

In Spree, there is this magical hidden file with a magical little line: https://github.com/spree/spree/blob/master/auth/app/controllers/resource_controller_decorator.rb#L3

"module ResourceController"

Although, I can't actually find straight evidence that this file is being required anywhere... but I reckon it's something like this is causing it. 

You can fix this by doing this:

class AdjustmentsController < Spree::Admin::ResourcesController

Or having it exactly the way it was originally:

class Spree::Admin::AdjustmentsController < Spree::Admin::ResourcesController

Well, that was fun. 

---- 

If anyone can find where that ResourceController module is *actually* coming from, I would be indebted to them. 

Thanks!

[Ryan Bigg]

Then I saw rd_resource_controller, and everything was answered. There's your culprit.

[Trung Le]

Thanks much for the great finding. Would removing rd_resource_controller help? This gem is there for the sake of backward comparability. I think it is about time to retire it.

@romul, @schof, @BDQ: what's your thoughts?

Trung LÊ