Skip to content

Created merge article feature for admins#33

Open
Dreedle wants to merge 28 commits intoAda-C4:masterfrom
Dreedle:master
Open

Created merge article feature for admins#33
Dreedle wants to merge 28 commits intoAda-C4:masterfrom
Dreedle:master

Conversation

@Dreedle
Copy link

@Dreedle Dreedle commented Mar 31, 2016

What I did do:

  • Original typo specs all green
  • Cucumber tests for
    ...merge feature is only available to admin
    ...merge feature is not available on new articles
    ...merge feature combines text of the two articles
    ...merge feature maintains title of just one article
    ...merge feature removes the other article
    -Deployed to heroku with this feature: https://aqueous-reaches-35072.herokuapp.com/

What I did not get to:

  • Making sure the user cannot merge the same article with itself
  • Error handling when an invalid article id is entered
  • Figuring out syntax on line 9 of _merge.html.erb to avoid repetition of other_article_id
  • Rspec tests for this feature
  • Cucumber tests for
    ...new article having a single author
    ...new article having comments from both articles

Dreedle added 28 commits March 28, 2016 13:45
…ditional to admin new categories controller (new or edit)
…est for bodies combining is currently failing
def new_or_edit
if params[:action] == "new"
@category = Category.new
elsif params[:action] == "edit" && params[:id]

Choose a reason for hiding this comment

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

Thought: Does the params[:action] in this elsif as well as the one below change the conditional, or could you just use params[:id]

Article ID
<%= text_field "other_article_id", "other_article_id", {:autocomplete => 'off', :style => 'width: 30%; margin-left: 10px'} %>
</span>
<div style="padding-top:10px">

Choose a reason for hiding this comment

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

Can you use a class here instead of inline style?

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