Add submission_url to config and refactor logic into services

[heratyian]

Added submission_url to the config. A lot of the logic in the rake tasks was hard to follow. Used Claude to break up logic from rake task into services for better separation of concerns. Did some smoke testing and seems to not break anything.

---

Important

Add submission_url configuration and refactor rake task logic into service classes for improved modularity and maintainability.

• Configuration:
  • Add submission_url to GradeRunner configuration in lib/grade_runner.rb.
  • Update README.markdown to document submission_url configuration.
• Refactoring:
  • Extract logic from rake tasks into service classes in lib/grade_runner/services/.
  • Introduce ConfigService, GithubService, GradeService, SpecService, and TokenService for modular functionality.
• Rake Tasks:
  • Refactor grade.rake and grade_runner.rake to use new service classes.
  • Simplify task logic by delegating to GradeService methods.
• Miscellaneous:
  • Update VERSION to 0.0.14.
  • Update Gemfile and grade_runner.gemspec for dependency management.
  • Minor updates to Rakefile and README.markdown for clarity and accuracy.

^{This description was created by for 89c6c86. It will automatically update as commits are pushed.}

[ellipsis-dev]

❌ Changes requested. Reviewed everything up to 89c6c86 in 2 minutes and 4 seconds

More details

• Looked at 1086 lines of code in 16 files
• Skipped 1 files when reviewing.
• Skipped posting 8 drafted comments based on config settings.

1. lib/grade_runner/runner.rb:7

• Draft comment:
  Consider handling potential trailing slash issues when concatenating submission_root_url and submission_path. Using URI.join or ensuring proper formatting can avoid double slashes.
• Reason this comment was not posted:
  Comment was on unchanged code.

2. lib/tasks/grade.rake:11

• Draft comment:
  Using ARGV.each to define tasks may be brittle and hard to maintain. It might be worth reviewing if this dynamic task creation is truly needed.
• Reason this comment was not posted:
  Comment was on unchanged code.

3. lib/grade_runner/runner.rb:7

• Draft comment:
  Consider normalizing the submission_url to avoid potential double slashes if submission_root_url ends with a trailing slash.
• Reason this comment was not posted:
  Comment was on unchanged code.

4. lib/grade_runner/services/github_service.rb:14

• Draft comment:
  Ensure ActiveSupport is required to use 'present?' here in non-Rails environments.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

5. lib/grade_runner/services/token_service.rb:28

• Draft comment:
  Consider logging or handling the error context in the rescue block for validate_token to aid debugging.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

6. lib/grade_runner/services/spec_service.rb:103

• Draft comment:
  Review the zip extraction logic; using 'unless File.exist?(file_path)' might skip extracting updates if a file already exists.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

7. lib/grade_runner/services/spec_service.rb:71

• Draft comment:
  Consider checking the outcome of the 'git commit' command to detect commit failures rather than suppressing output.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50%
  None

8. lib/grade_runner/services/token_service.rb:51

• Draft comment:
  Consider updating the message 'Please enter valid token' to 'Please enter a valid token' for better clarity.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  While the suggestion would make the message more grammatically correct, this is an extremely minor UI text change. The current message is perfectly understandable. The rules specifically say not to comment on pure UI changes and to only keep comments that require clear code changes. This feels like unnecessary nitpicking that doesn't materially improve the code.
  The message could be seen as slightly unprofessional without proper grammar. Some might argue that maintaining high quality even in error messages is important.
  While polish is good, this change is too minor to warrant a PR comment. The current message is clear enough for users to understand.
  Delete this comment as it's about a minor UI text change that doesn't impact functionality or code quality in any meaningful way.

Workflow ID: wflow_yM0IrLgypYoWPmkG

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

Rescue in validate_token swallows errors without logging; consider logging the exception for easier debugging.

[heratyian]

cc: @bpurinton @raghubetina

let me know if this is something you'd like to use.

TLDR; lots of refactoring to improve readability and added submission_url to config.

[bpurinton]

@heratyian, wow, thanks for this work! We will need to do some extensive testing with Grades + grade_runner in a bunch of projects to make sure this works as expected and allows the usual communication. (That setup is somewhat involved, but something we do frequently.)

I am curious about your motivation for adding the submission_url config. Do you intend to host results outside of Grades? I'm not sure how that would work off the top of my head, since I think grade_runner and Grades are rather intertwined.

cc @jelaniwoods, you may also want to take a first look at this and comment any questions or concerns.

[heratyian]

I am curious about your motivation for adding the submission_url config. Do you intend to host results outside of Grades? I'm not sure how that would work off the top of my head, since I think grade_runner and Grades are rather intertwined.

Correct. I've created a dpi grades server and it's working. Unfortunately I'm getting pushed to ween dpi off first draft over the next year or so.

In general I really like this unit test suite grade passback in a codespace approach since students are using the tools of the trade and I can track progress in canvas. I'd like to build out more projects using grade_runner (maybe even create something for other languages) Ideally we can still work together on some open source tools like this.

[bpurinton]

Gotcha. We had a look at the code changes and discussed this. Since these are pretty significant changes, and since reviewing and testing them on our side takes a lot of time that we don't have right now on our small team, we'd prefer that for your purposes you maintain a fork of the grade_runner gem with these updates to use a custom domain for submissions.

I think it's worth keeping this PR open for now as a reminder to us, and as a potentially useful refactoring of the gem (but in need of a thorough review), if you'd like.

[heratyian]

I think it's worth keeping this PR open for now as a reminder to us, and as a potentially useful refactoring of the gem (but in need of a thorough review), if you'd like.

Appreciate it. fwiw I haven't tested override_local_specs feature thoroughly or with any rails/sinatra projects yet (just ruby scripts). I have some ideas on further simplification and refactoring. I'll continue working off fork and keep ya'll updated.