Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PR Submission #14

Open
wants to merge 51 commits into
base: jlnmmr/master
Choose a base branch
from
Open

PR Submission #14

wants to merge 51 commits into from

Conversation

noglows
Copy link

@noglows noglows commented Nov 20, 2015

Requirements complete. There are still some CSS issues and areas to DRY. There are also areas we know could be improved (ex: moving code from views to the controller and simplifying controller code).

noglows and others added 30 commits November 17, 2015 16:39
@noglows
Seeded data for markets and vendors
44371ac
@noglows
All markets being displayed on /markets
9fa7ce1
@noglows
Added ability to view details for a selected market
fbe5bb5
@noglows
Add a new market form complete
678c4ee
@knaydee
Add ability to create new market
ef9d69c
@knaydee
Add create and edit Market
3cc449a
@noglows
Market page shows vendors
ba2ef4e
@noglows
Display a vendor list for a market
6efbf87
@noglows
Started work on adding new vendors
ae7be00
@knaydee
Add ability to add a new vendor to a market
cadae48
@noglows
Ability to edit vendors
98325ac
@noglows
Deleting vendors added
d052787
@knaydee
Sign in as vendor is working and fixed vendor routing
8153ccc
@noglows
Fixed the routes for editing and deleting vendors as a market
e21d303
@noglows
Re-added in the Add Vendor button
346cc3b
@knaydee
Things are broken :(
89b5421
@knaydee
Partial bug fix
3053e6f
@noglows
Fixed more of the route issues
0972659
@knaydee
Adding a new vendor fix
99fe572
@knaydee
Seed sales and products data
9ddfcfc
@knaydee
Create show.html.erb for Vendor with sales and products buttons and r…
8f9853a 
…outes
@noglows
Displaying vendor sales
592f97c
@noglows
Showing product name next to sales
a3e183d
@knaydee
Add index for products
910b2f7
@knaydee
Add products destroy and started edit
84f4537
@knaydee
Add ability to add a new product to a vendor
0d4d5d9
@knaydee
Add ability to edit vendor products
34d6348
@noglows
Sales total shown on Vendor sales page
4cb2603
@noglows
Added the ability to add a sale
efd1f27
@knaydee
Add a new relationship and list sales for the month
c1b5efd
kariabancroft
kariabancroft reviewed Nov 24, 2015
View reviewed changes
app/controllers/markets_controller.rb

def edit
id = params[:id]
@market = Market.find(id)
Copy link

Choose a reason for hiding this comment

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

I think it would be great to see the id and the @market variable assignment moved into a before_action for these specific methods.

@kariabancroft
Copy link

Nice job on this @noglows & @knaydee! Be cautious and avoid putting styles directly into the views and to only use instance variables when necessary. I think you can clean up the controllers a bit, but overall you did a nice job of using strong params and the active record objects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@noglows @kariabancroft @knaydee