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

Action items -> Needs attention auctions #987

Merged
merged 1 commit into from Aug 2, 2016
Merged

Conversation

adelevie
Copy link
Contributor

@adelevie adelevie commented Aug 2, 2016

Lays the groundwork for #939 and the other "Needs attention" stories.

@adelevie adelevie force-pushed the ad-attention-needed branch 2 times, most recently from 6c8e0ee to 73d2d95 Compare August 2, 2016 15:30
@@ -0,0 +1,5 @@
class Admin::NeedsAttentionAuctionsController < Admin::BaseController
Copy link
Contributor

Choose a reason for hiding this comment

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

thoughts on Admin::Auctions::NeedsAttentionControllerto be more consistent with https://github.com/18F/micropurchase/blob/develop/app/controllers/admin/auctions/closed_controller.rb ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess so! good call!

@jessieay
Copy link
Contributor

jessieay commented Aug 2, 2016

One naming question otherwise LGTM! Nice thinking getting this all set up before moving into new functionality

@@ -0,0 +1,5 @@
class Admin::Auctions::NeedsAttentionController < Admin::BaseController
def index
@view_model = Admin::NeedsAttentionAuctionsViewModel.new
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jessieay should these be renamed, too? to something like Admin::Auctions::NeedsAttentionViewModel?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I did not do that for the other controller but probably makes sense. If you do it, mind also renaming https://github.com/18F/micropurchase/blob/develop/app/view_models/admin/closed_auctions_list_item_view_model.rb ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problemo

On Tuesday, August 2, 2016, Jessie A. Young notifications@github.com
wrote:

In app/controllers/admin/auctions/needs_attention_controller.rb
#987 (comment):

@@ -0,0 +1,5 @@
+class Admin::Auctions::NeedsAttentionController < Admin::BaseController

  • def index
  • @view_model = Admin::NeedsAttentionAuctionsViewModel.new

yeah I did not do that for the other controller but probably makes sense.
If you do it, mind also renaming
https://github.com/18F/micropurchase/blob/develop/app/view_models/admin/closed_auctions_list_item_view_model.rb
?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/18F/micropurchase/pull/987/files/358f90120da907053c6e0f39e7bb1291c42e3871#r73193822,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFTBq1aEURip_BsNYXiyMvoot6JZj8Yks5qb3ZNgaJpZM4JatzG
.

@jessieay
Copy link
Contributor

jessieay commented Aug 2, 2016

Going to merge as-is -- we can rename those view models separately!

@jessieay jessieay merged commit 05d6cce into develop Aug 2, 2016
@jessieay jessieay deleted the ad-attention-needed branch August 2, 2016 18:21
@adelevie
Copy link
Contributor Author

adelevie commented Aug 2, 2016

Thanks!

On Tuesday, August 2, 2016, Jessie A. Young notifications@github.com
wrote:

Going to merge as-is -- we can rename those view models separately!


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#987 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFTBi6OF0kftVIbwd1Va28kqVsjQnbeks5qb4rCgaJpZM4JatzG
.

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.

None yet

2 participants