Skip to content
This repository has been archived by the owner on Oct 2, 2019. It is now read-only.

[#89168592] create a Proposal model #127

Merged
merged 19 commits into from
Feb 26, 2015
Merged

[#89168592] create a Proposal model #127

merged 19 commits into from
Feb 26, 2015

Conversation

afeld
Copy link
Contributor

@afeld afeld commented Dec 15, 2014

I have been thinking about how to generalize the app, and short of actually splitting the application into multiple pieces, this PR splits a Proposal model out from Cart. The medium-term goal is to separate any GSA Advantage-specific logic from that dealing with the overall workflow and approvals. After this, I plan to create a new API for Proposals (not Carts), which can be used by Navigator, WHSC, etc.

@afeld afeld mentioned this pull request Dec 16, 2014
Conflicts:
	spec/controllers/communicarts_controller_spec.rb
@@ -0,0 +1,13 @@
class Proposal < ActiveRecord::Base
has_one :cart
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an equivalent that'd allow zero or one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, has_one does that, though probably worth noting that it won't always be present in an inline comment. Good call!

@afeld
Copy link
Contributor Author

afeld commented Feb 18, 2015

Had a couple train rides worth of downtime – updated!

@cmc333333
Copy link
Contributor

Seems like a good step towards your goal. Let me know if I can help integrating this with the workflow piece.


dir.up do
Cart.find_each do |cart|
cart.create_proposal!(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that it's keeping the same id on the carts, so none of the other references need to change.

@afeld
Copy link
Contributor Author

afeld commented Feb 25, 2015

Phew, ok, updated. Feels like this was a lot of work for what it was, but I am still fairly confident about the direction of having a Proposal model and then distinct models for each use case. Someone mind taking another look-see?

@cmc333333
Copy link
Contributor

👍 from me -- I'd like to see this merged soon so it doesn't drift too far from master. I don't feel confident enough to merge myself, though.

@phirefly - when you get a chance, review and merge?

@phirefly
Copy link
Contributor

We'll probably need a backfill script. Any in the works?

t.string :flow
t.timestamps
end
add_reference :carts, :proposal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will NCR be of type Cart? Would it be worthwhile to name that appropriately at this point? I'm also happy with capturing that in a separate story.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the moment, yes. My plan was to deal with splitting the Cart into use-case-specific models separately.

@afeld
Copy link
Contributor Author

afeld commented Feb 26, 2015

Updated! Not sure if I succeeded in simplifying anything though 😕 To minimize the number of changes, I left all the state-related calls to the Cart (approve!, rejected?, etc.), but this required jumping through some hoops to get the delegation to work. Moving the Approval-related stuff (the relationships and the methods) to the Proposal later will help.

@afeld afeld changed the title create a Proposal model [#89168592] create a Proposal model Feb 26, 2015
@phirefly
Copy link
Contributor

:shipit:

phirefly added a commit that referenced this pull request Feb 26, 2015
[#89168592] create a Proposal model
@phirefly phirefly merged commit b7fb0e3 into master Feb 26, 2015
@phirefly phirefly deleted the proposals branch February 26, 2015 17:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants