Build trusted publisher filter

[justinddaniel]

No description provided.

[simi]

Doesn't trusted_publisher? already return true/false? Is there any reason for explicit ? true: false?

[justinddaniel]

The safe navigator means a nil value can be returned if the version has no pusher_api_key.

[simi]

Would pusher_api_key? && pusher_api_key.trusted_publisher? work also?

[justinddaniel]

No, but pusher_api_key.present? && pusher_api_key.trusted_publisher? would work. It is a matter of preference here.

[simi]

Or maybe pusher_api_key&.trusted_publisher? || false. Yes, just a nitpick indeed.

[martinemde]

Am I the only one that prefers !! to force boolean?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.19%. Comparing base (a86b845) to head (ecb6a85).
Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5237      +/-   ##
==========================================
- Coverage   96.85%   94.19%   -2.67%     
==========================================
  Files         456      456              
  Lines        9517     9578      +61     
==========================================
- Hits         9218     9022     -196     
- Misses        299      556     +257     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[justinddaniel]

Linked issue: #4807

[simi]

CI seems unhappy about this test.

rubygems.org/test/models/pusher_test.rb

Lines 632 to 659 in 1143eba

	expected_response = {
	"name" => "gemsgemsgems",
	"downloads" => 0,
	"version" => "0.1.1",
	"version_downloads" => 0,
	"platform" => "ruby",
	"authors" => "Joe User",
	"info" => "Some awesome gem",
	"licenses" => "MIT",
	"metadata" => { "foo" => "bar" },
	"sha" => "b5d4045c3f466fa91fe2cc6abe79232a1a57cdf104f7a26e716e0a1e2789df78",
	"project_uri" => "http://localhost/gems/gemsgemsgems",
	"gem_uri" => "http://localhost/gems/gemsgemsgems-0.1.1.gem",
	"homepage_uri" => "http://example.com",
	"wiki_uri" => "http://example.com",
	"documentation_uri" => "http://example.com",
	"mailing_list_uri" => "http://example.com",
	"source_code_uri" => "http://example.com",
	"bug_tracker_uri" => "http://example.com",
	"changelog_uri" => nil,
	"funding_uri" => nil,
	"yanked" => false,
	"summary" => "old summary",
	"description" => "Some awesome gem",
	"updated" => "2016-07-04T00:00:00.000Z",
	"dependencies" => { "development" => [], "runtime" => [] },
	"suggest" => { "input" => "gemsgemsgems", "weight" => 0, "contexts" => { "yanked" => false } }
	}

Should be as simple fix as adding this new key in to the tested hash.

[simi]

Does this mapping change need full re-indexing?

[justinddaniel]

Good question. I would not think so, since there is no change to the database source schema, but I am not sure. It is my first time committing to this repo and I do not have full context, so I will wait for others to weigh in.

[colby-swandale]

Any new mappings do require a reindex, which is easy to do but needs some coordination when deployed.

[martinemde]

@justinddaniel Thank you for contributing this! Pleasure working with you today.