Skip to content

Conversation

benhalpern
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Documentation Update

Description

This feature should help folks highlight their most impactful stuff, while also being less hesitant to post simpler discussion/help threads etc. without pushing down their best content.

@pr-triage pr-triage bot added the PR: unreviewed bot applied label for PR's with no review label Jun 23, 2019
redirect_back(fallback_location: "/dashboard")
end

def update
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 are reason why this is not the destroy action?

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'd need to look further into whether we have addressed this in updates since the fact, but going way back to this project's creation I removed the jquery dependency for Rails that was actually required for calling destroy actions. I know Rails has since removed jquery as a dependency from the ujs approach but I believe it is still the case that we can't call destroy actions in the Rails way without importing the specific dependencies, which we do on the "are you sure you want to destroy this" page.

We should definitely make get this documented and make some of these overall concepts around how we cache pages more clearly documented in our Readme/docs.

Anyway, I figured in and of itself update was close enough to what's going on with updating the state of something's pinned status that it's clean enough for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

@benhalpern there's an example in the code that does something similar I guess. If you look at the code for removing the API key:

https://github.com/thepracticaldev/dev.to/blob/dcbb188cf1e3ddbbf87711a43837766eb609676b/app/views/users/_account.html.erb#L47-L51

that's a POST with the method: :delete trick that ends up in the destroy action inside the api secrets controller https://github.com/thepracticaldev/dev.to/blob/master/app/controllers/api_secrets_controller.rb#L20

@@ -22,6 +22,7 @@ class Article < ApplicationRecord
counter_culture :organization

has_many :comments, as: :commentable, inverse_of: :commentable
has_many :profile_pins, as: :pinnable, inverse_of: :pinnable
Copy link
Contributor

@rhymes rhymes Jun 23, 2019

Choose a reason for hiding this comment

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

Why does an article has many profile_pins? I don't understand. Shouldn't an article have at most one pin? Is this to allow the same article to be pinned by multiple people in an organization?

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 didn't build in the full functionality yet, but created the structure so that one article could be simultaneously pinned to a user, an organization and any of its tags. Each acts as a "profile" in this relationship.

@benhalpern benhalpern merged commit dcbb188 into master Jun 24, 2019
@pr-triage pr-triage bot added PR: merged bot applied label for PR's that are merged PR: unreviewed bot applied label for PR's with no review and removed PR: unreviewed bot applied label for PR's with no review PR: merged bot applied label for PR's that are merged labels Jun 24, 2019
@rhymes rhymes deleted the ben/pins branch June 24, 2019 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: unreviewed bot applied label for PR's with no review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants