Don't recreate the same channel due to https errors. Closes #1348

[yachtcaptain23]

Submitter Checklist:

• Submitted a ticket for my issue if one did not already exist.
• Used Github auto-closing keywords in the commit message.
• Added/updated tests for this change (for new code or code which already has tests).
• Tagged reviewers.
• Integrated piwik/matomo (for code that adds new buttons).
• Addressed or ignored all brakeman warnings

Test Plan:

Reviewer Checklist:

Tests

• Adequate test coverage exists to prevent regressions

Security:

• No raw SQL -- Always prefer ActiveRecord query helpers (more info on StackOverflow)
• XSS is mitigated -- Avoid html_safe and raw; escape untrusted content from users and 3rd party APIs (Rails XSS guide and OWASP XSS Prevention Cheat Sheet)

[nvonpentz]

Not sure if this is completed yet, but I'm still able to reproduce brave-intl/creators-private-issues#845 on this branch

[corymcdonald]

I was investigating what to do in that case because really what happens is that the domain normalizer strips everything and goes to username.github.io which calls the fetch call to return 404.

So we'd need to write some code to handle that in this method
https://github.com/brave-intl/publishers/blob/staging/app/services/site_channel_host_inspector.rb#L46

EDIT: Whoops, I was thinking of a different issue 😅

[yachtcaptain23]

Spoke with Nick

Action items:
(1) Get rid of the abandoned scope on SiteChannelDetails
(2) Update the code in app/jobs/clean_abandoned_site_channels_job.rb
(3) Re-insert back original controller logic.

[corymcdonald]

can we change this to
raise if channel.details.verification_method.present?
i think that's slightly more readable

[corymcdonald]

Hey Albert, I don't think this actually resolves the issue, here's a gif to reproduce it

[yachtcaptain23]

@corymcdonald I agree that this solution doesn't explain the whole story. The issue's wording should be restructured, as the issue you're seeing might not be an issue, and I thought it had tied into another issue in Sentry. I think we should continue to allow users to make channels that haven't fulfilled the flow (it's a tradeoff between refactoring a bunch of logic and inheriting more risk or maintain a confusing mental model of visible and non-visible/abandoned channels 😞 )

I originally intended to solve https://sentry.io/brave-software/publishers-production-t2/issues/761612367/?query=is:unresolved and brave-intl/creators-private-issues#845 in this PR. However, they seem to be separate problems: the sentry error was 1 simple bad channel in which I fixed. This PR solves some weird state called abandoned when it should have just been non-visible

To elaborate further on the Sentry error: We have this constraint that prevents a publisher from creating duplicates (and somehow, someone got around it. I don't think it's a race condition though):

  def site_channel_details_brave_publisher_id_unique_for_publisher
    duplicate_unverified_channels = publisher.channels.visible_site_channels
                                                     .where(site_channel_details: { brave_publisher_id: self.details.brave_publisher_id })
                                                     .where.not(id: id)

    if duplicate_unverified_channels.any?
      errors.add(:brave_publisher_id, "must be unique")
    end
  end

Anyways, an elegant solution to solve brave-intl/creators-private-issues#845 for a dropped funnel, is to allow the user to continue from where they left off. However, there's several channels that are abandoned and not meant to be visible. Overall the state transitions for adding a channel are confusing, but they haven't caused any major issues, yet (except for us developers).

[nvonpentz]

should be resolved now, the verification method wasn't being set which is needed to make a channel visible