Extracting listing routes (#16338)
The fundamental goal in this refactor is to make it easier to disable the
Listings feature set. By extracting a routes file (via the `draw` method [see
implementation details][1]), we can more easily disable those routes _en
masse_. This "crease in the code" could mean that we wouldn't need to update
the controller action logic. An update of that logic might mean going into
each controller action related to listing and adding a conditional for render
404 when listing is disabled.
Now, on to why this should work (not I did not open the application and click
around, but instead went with a "let the framework help me" mindset).
I believe that this should work; though there's a caveat†. Here's why I
believe this will work for extracting the listing routes:
Before making changes I ran the following:
```sh
$ rails routes | sort > original-routes.txt
```
Let's break that down:
- `rails routes` :: renders the routing table for Rails; this is how
Rails converts routes to the magic of stuff like `article_path(:id)`
- `| sort` :: to sort the results
- `> original-routes.txt` :: to write the results into a file.
Then I made the changes and ran the following:
```sh
$ rails routes | sort > updated-listing-routes.txt
```
And finally, I ran:
```sh
$ diff updated-routes.txt original-routes.txt
```
The results were as follows:
```sh
500c500
< api_organization_listings GET (/locale/:locale)/api/organizations/:organization_username/listings(.:format) api/v0/organizations#listings {:locale=>nil, :format=>"json", :to=>"organizations#listings"}
---
> api_organization_listings GET (/locale/:locale)/api/organizations/:organization_username/listings(.:format) api/v0/organizations#listings {:locale=>nil, :format=>"json"}
```
Note the only difference is that the above two lines, one has
`:to=>"organizations#listings"` and the other does not. However, the
`./spec/requests/api/v0/organizations_spec.rb` exercises the route; so I
assume a hiccup/foible in the route reporting.
There was no difference between the before and after files.
† Caveat: if for some reason the order of the routes mattered, this will
have messed that up. I don't think that is the case, but we shall see.
Related to forem/rfcs#291 and #16335
This commit is contained in:
Jeremy Friesen
GitHub
parent
77d9640e3e
commit
6fb7361552
4 changed files with 35 additions and 19 deletions
|
|
@ -26,6 +26,7 @@ Rails.application.routes.draw do
|
|||
get "/locale/:locale", to: "stories#index"
|
||||
|
||||
draw :admin
|
||||
draw :listing
|
||||
|
||||
namespace :stories, defaults: { format: "json" } do
|
||||
resource :feed, only: [:show] do
|
||||
|
|
@ -36,8 +37,7 @@ Rails.application.routes.draw do
|
|||
end
|
||||
|
||||
namespace :api, defaults: { format: "json" } do
|
||||
scope module: :v0,
|
||||
constraints: ApiConstraints.new(version: 0, default: true) do
|
||||
scope module: :v0, constraints: ApiConstraints.new(version: 0, default: true) do
|
||||
resources :articles, only: %i[index show create update] do
|
||||
collection do
|
||||
get "me(/:status)", to: "articles#me", as: :me, constraints: { status: /published|unpublished|all/ }
|
||||
|
|
@ -65,8 +65,6 @@ Rails.application.routes.draw do
|
|||
end
|
||||
resources :readinglist, only: [:index]
|
||||
|
||||
resources :listings, only: %i[index show create update]
|
||||
get "/listings/category/:category", to: "listings#index", as: :listings_category
|
||||
get "/analytics/totals", to: "analytics#totals"
|
||||
get "/analytics/historical", to: "analytics#historical"
|
||||
get "/analytics/past_day", to: "analytics#past_day"
|
||||
|
|
@ -83,7 +81,6 @@ Rails.application.routes.draw do
|
|||
resources :profile_images, only: %i[show], param: :username
|
||||
resources :organizations, only: [:show], param: :username do
|
||||
resources :users, only: [:index], to: "organizations#users"
|
||||
resources :listings, only: [:index], to: "organizations#listings"
|
||||
resources :articles, only: [:index], to: "organizations#articles"
|
||||
end
|
||||
resource :instance, only: %i[show]
|
||||
|
|
@ -156,7 +153,6 @@ Rails.application.routes.draw do
|
|||
resources :tag_adjustments, only: %i[create destroy]
|
||||
resources :rating_votes, only: [:create]
|
||||
resources :page_views, only: %i[create update]
|
||||
resources :listings, only: %i[index new create edit update destroy dashboard]
|
||||
resources :credits, only: %i[index new create] do
|
||||
get "purchase", on: :collection, to: "credits#new"
|
||||
end
|
||||
|
|
@ -193,17 +189,9 @@ Rails.application.routes.draw do
|
|||
|
||||
get "/verify_email_ownership", to: "email_authorizations#verify", as: :verify_email_authorizations
|
||||
get "/search/tags", to: "search#tags"
|
||||
get "/search/listings", to: "search#listings"
|
||||
get "/search/usernames", to: "search#usernames"
|
||||
get "/search/feed_content", to: "search#feed_content"
|
||||
get "/search/reactions", to: "search#reactions"
|
||||
get "/listings/dashboard", to: "listings#dashboard"
|
||||
get "/listings/:category", to: "listings#index", as: :listing_category
|
||||
get "/listings/:category/:slug", to: "listings#index", as: :listing_slug
|
||||
get "/listings/:category/:slug/:view", to: "listings#index",
|
||||
constraints: { view: /moderate/ }
|
||||
get "/listings/:category/:slug/delete_confirm", to: "listings#delete_confirm"
|
||||
delete "/listings/:category/:slug", to: "listings#destroy"
|
||||
get "/notifications/:filter", to: "notifications#index", as: :notifications_filter
|
||||
get "/notifications/:filter/:org_id", to: "notifications#index", as: :notifications_filter_org
|
||||
get "/notification_subscriptions/:notifiable_type/:notifiable_id", to: "notification_subscriptions#show"
|
||||
|
|
@ -221,7 +209,6 @@ Rails.application.routes.draw do
|
|||
get "/social_previews/user/:id", to: "social_previews#user", as: :user_social_preview
|
||||
get "/social_previews/organization/:id", to: "social_previews#organization", as: :organization_social_preview
|
||||
get "/social_previews/tag/:id", to: "social_previews#tag", as: :tag_social_preview
|
||||
get "/social_previews/listing/:id", to: "social_previews#listing", as: :listing_social_preview
|
||||
get "/social_previews/comment/:id", to: "social_previews#comment", as: :comment_social_preview
|
||||
|
||||
get "/async_info/base_data", controller: "async_info#base_data", defaults: { format: :json }
|
||||
|
|
@ -264,7 +251,6 @@ Rails.application.routes.draw do
|
|||
|
||||
# These routes are required by links in the sites and will most likely to be replaced by a db page
|
||||
get "/about", to: "pages#about"
|
||||
get "/about-listings", to: "pages#about_listings"
|
||||
get "/security", to: "pages#bounty"
|
||||
get "/community-moderation", to: "pages#community_moderation"
|
||||
get "/faq", to: "pages#faq"
|
||||
|
|
|
|||
|
|
@ -156,9 +156,6 @@ namespace :admin do
|
|||
|
||||
scope :apps do
|
||||
resources :consumer_apps, only: %i[index new create edit update destroy]
|
||||
resources :listings, only: %i[index edit update destroy]
|
||||
resources :listing_categories, only: %i[index edit update new create
|
||||
destroy], path: "listings/categories"
|
||||
resources :welcome, only: %i[index create]
|
||||
end
|
||||
end
|
||||
|
|
|
|||
27
config/routes/listing.rb
Normal file
27
config/routes/listing.rb
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
namespace :admin do
|
||||
scope :apps do
|
||||
resources :listings, only: %i[index edit update destroy]
|
||||
resources :listing_categories, only: %i[index edit update new create
|
||||
destroy], path: "listings/categories"
|
||||
end
|
||||
end
|
||||
|
||||
namespace :api, defaults: { format: "json" } do
|
||||
scope module: :v0, constraints: ApiConstraints.new(version: 0, default: true) do
|
||||
resources :listings, only: %i[index show create update]
|
||||
get "/listings/category/:category", to: "listings#index", as: :listings_category
|
||||
get "/organizations/:organization_username/listings", to: "organizations#listings",
|
||||
as: :organization_listings
|
||||
end
|
||||
end
|
||||
resources :listings, only: %i[index new create edit update destroy dashboard]
|
||||
|
||||
get "/search/listings", to: "search#listings"
|
||||
get "/listings/dashboard", to: "listings#dashboard"
|
||||
get "/listings/:category", to: "listings#index", as: :listing_category
|
||||
get "/listings/:category/:slug", to: "listings#index", as: :listing_slug
|
||||
get "/listings/:category/:slug/:view", to: "listings#index", constraints: { view: /moderate/ }
|
||||
get "/listings/:category/:slug/delete_confirm", to: "listings#delete_confirm"
|
||||
delete "/listings/:category/:slug", to: "listings#destroy"
|
||||
get "/social_previews/listing/:id", to: "social_previews#listing", as: :listing_social_preview
|
||||
get "/about-listings", to: "pages#about_listings"
|
||||
|
|
@ -82,6 +82,12 @@ RSpec.describe "Api::V0::Organizations", type: :request do
|
|||
expect(response).to have_http_status(:not_found)
|
||||
end
|
||||
|
||||
it "returns success for when orgnaization username exists" do
|
||||
create(:listing, user: org_user, organization: organization)
|
||||
get "/api/organizations/#{organization.username}/listings"
|
||||
expect(response).to have_http_status(:success)
|
||||
end
|
||||
|
||||
it "supports pagination" do
|
||||
create(:listing, user: org_user, organization: organization)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue