fix: with_foreign_key modifier fails with wrong error message

[vsppedro]

Closes #1374

I noticed that for the scenario described on issue #1374 the class_has_foreign_key?(klass) was entering in the first conditional and than running this piece of code:

option_verifier.correct_for_string?(
  :foreign_key,
  options[:foreign_key],
)

It seems to me that the problem with this flow is that it just returns false without updating the failure message. What do you think?

[vsppedro]

There is something wrong with the Run linters action:

Error: Error trying to create GitHub check for RuboCop: Received status code 403
    at createCheck (/home/runner/work/_actions/wearerequired/lint-action/v1/src/github/api.js:64:9)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
/home/runner/work/_actions/wearerequired/lint-action/v1/src/index.js:13
	throw new Error(`Exiting because of unhandled promise rejection`);
	^

Error: Exiting because of unhandled promise rejection
    at process.<anonymous> (/home/runner/work/_actions/wearerequired/lint-action/v1/src/index.js:13:8)
    at process.emit (events.js:210:5)
    at processPromiseRejections (internal/process/promises.js:201:33)
    at processTicksAndRejections (internal/process/task_queues.js:94:32)

I will try to understand the problem.

[mcmire]

Hmm, it seems to be related to this problem: wearerequired/lint-action#13. I think since you have commit access now you should be able to push a branch directly to the repo instead of making a PR from your fork. That may be a solution for now.

[mcmire]

I'm not sure why this bug wasn't caught before but your fix seems right to me!

[vsppedro]

I see. For the next PR I will not use my fork anymore. Thanks for the suggestion.

So, for now, if anyone opens a PR we will need to ask them to run the rubocop locally just to make sure that they're not adding any offense.

I did it on this PR and there is no offense. Can I merge it or do you prefer that I open a new PR not using my fork?

[KapilSachdev]

So, for now, if anyone opens a PR we will need to ask them to run the rubocop locally just to make sure that they're not adding any offense.

That will defeat the purpose of automating it.

I'm not sure why this bug wasn't caught

I was aware of the issue but i didn't anticipated this effect. I misunderstood that issue in some way thinking that the token was of the fork so it would not add the annotation on the destination repo, but once #1371 is merged this will not happen.

@mcmire what do you think about removing lint-action? This issue arises because github does not allows it so we don't know the timeline of this fix.
We can add a custom github action for rubocop, but without annotation (eg https://github.com/rails/rails/runs/1403456898).
Initially i was into adding this but we were interested in annotation so this was cancelled.

OR we can try this wearerequired/lint-action#13 (comment).
Actually i tried this while working on #1371 , but it still failed. I will give it a try and if it works we can use that.

[KapilSachdev]

pull_request_target does not looks like a reliable fix either.
I created a PR in my fork from another fork and the rubocop action failed due to a reason reported in that same thread of lint-action and there does not seems to be any response against it.

So for now using a custom rubocop lint action without annotation seems a reasonable compromise as hound does not support rubocop > 0.91

@mcmire Thoughts?

[mcmire]

@KapilSachdev Dang, that's annoying. Well, okay, I guess a lint action that doesn't support annotations is better than not having one. So I'd be fine with that.

[vsppedro]

I noticed that a fix for the CI was recently added on master. I'll rebase this branch.

[KapilSachdev]

Yes, the annotation based rubocop linting is replaced with a simple workflow to check rubocop offences.

[vsppedro]

It worked. Merging. Thanks!