Skip to content

Conversation

shuaitseng
Copy link

@shuaitseng shuaitseng commented Oct 10, 2025

  • New CRUD operations for Route Plans

Important

Adds route plans endpoint with CRUD operations and removes outdated comments across multiple files.

  • Route Plans Endpoint:
    • Adds routeplans.rb with CRUD operations: create, get, get_by_id, update, add_tasks_to_routeplan, delete_one.
    • Integrates routeplans.rb into onfleet.rb.
  • Code Cleanup:
    • Removes outdated "ACTION: still needs to be tested" comments in destinations.rb, organizations.rb, recipients.rb, tasks.rb, teams.rb, webhooks.rb, workers.rb.
    • Updates get_unassigned_tasks in teams.rb to include optional query parameters.

This description was created by Ellipsis for 45409ed. You can customize this summary. It will automatically update as commits are pushed.

Copy link

ellipsis-dev bot commented Oct 10, 2025

PR Summary

This pull request adds support for the Route Plans API endpoints to the Ruby Onfleet SDK. The changes include:

  • Added a new resource file lib/resources/routeplans.rb that implements the Onfleet::Routeplans class
  • Implemented CRUD operations for Route Plans:
    • Create route plans
    • Retrieve route plans (by query and by ID)
    • Update route plans
    • Delete route plans
  • Added a method to add tasks to a specific route plan
  • Updated the main lib/onfleet.rb file to require the new routeplans resource

These changes allow Ruby developers to interact with Onfleet's Route Plans API using the SDK, completing the API coverage for this resource type.

Copy link

ellipsis-dev bot commented Oct 10, 2025

⚠️ This PR modifies application logic but lacks adequate unit test coverage. Please ensure all new logic is tested.

I notice that this PR adds new CRUD operations for Route Plans in the Ruby library, but there don't appear to be any corresponding test files added or modified. Since this is adding new application logic (API endpoints), it would be beneficial to include unit tests to verify the functionality works as expected and continues to work in the future.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed everything up to 0db74c2 in 1 minute and 39 seconds. Click for details.
  • Reviewed 71 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/resources/routeplans.rb:39
  • Draft comment:
    Inconsistent indentation: most methods indent inner lines with 2 extra spaces (total 6 relative to the class), but this method’s inner lines are indented further. Please adjust to match the rest of the file.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The comment points out a real formatting inconsistency. However, this is a style/formatting issue that would typically be caught and fixed automatically by code formatters or linters. The rules state we should not make purely informative comments or comments about obvious issues. The indentation inconsistency is real and could make the code less readable. Maybe this codebase doesn't have automated formatting tools in place? Even if there are no automated tools, indentation issues are obvious visual problems that don't require a comment to point out. The PR author can see this directly. This comment should be deleted as it points out an obvious formatting issue that doesn't require explanation.
2. lib/resources/routeplans.rb:40
  • Draft comment:
    The path string here includes a leading slash, unlike other endpoints. For consistency, remove the leading slash from the route path.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_72Oj3oQ5ac9bmJMt

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 6a18af4 in 1 minute and 20 seconds. Click for details.
  • Reviewed 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/onfleet.rb:17
  • Draft comment:
    This change appears to be a no-op (the require for './resources/workers' is unchanged). Please remove any redundant whitespace or non-functional modifications.
  • 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% The comment is technically correct - there is trailing whitespace added that serves no purpose. However, this is an extremely minor formatting issue that would likely be caught by linters or formatters. The comment doesn't point out any real code issues or suggest meaningful improvements. The comment is factually accurate. Perhaps keeping it could help maintain consistent code formatting. While accurate, comments about minor whitespace issues are too trivial and would create noise in the review. This should be handled by automated tools rather than manual comments. Delete this comment as it points out an extremely minor formatting issue that doesn't impact functionality and would be better handled by automated tools.
2. lib/onfleet.rb:17
  • Draft comment:
    There's an extra trailing space at the end of the line (require_relative './resources/workers' ). Consider removing the trailing whitespace.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While trailing whitespace is generally undesirable, this is an extremely minor stylistic issue that would typically be handled by editor settings or linting rules. It doesn't affect functionality and isn't worth a PR comment. Most teams have automated tools to handle whitespace issues. Perhaps trailing whitespace could cause issues in some edge cases or with certain tools? Maybe the team has specific formatting standards they want to enforce manually? Even if there are edge cases, this is still too minor for a manual review comment. Such formatting issues should be caught by automated tools, not human reviewers. This comment should be removed as it addresses an extremely minor stylistic issue that would be better handled by automated tooling.

Workflow ID: wflow_fhhdnfbEQnD3OzFn

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed cc11878 in 38 seconds. Click for details.
  • Reviewed 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/resources/routeplans.rb:40
  • Draft comment:
    The path now omits the leading slash to be consistent with other endpoint definitions. Please ensure that Onfleet.request expects a relative path. This change aligns with the module's overall URL pattern.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 30% <= threshold 50% The comment is asking the PR author to ensure that Onfleet.request expects a relative path, which violates the rule against asking the author to confirm or ensure behavior. However, it also provides a suggestion about aligning with the module's URL pattern, which is useful. The comment is partially useful but mostly violates the rules.

Workflow ID: wflow_wInsXT29ISQZe4tB

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Changes requested ❌

Reviewed efd9942 in 2 minutes and 12 seconds. Click for details.
  • Reviewed 212 lines of code in 9 files
  • Skipped 0 files when reviewing.
  • Skipped posting 8 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/onfleet.rb:17
  • Draft comment:
    Removed trailing whitespace from the require statement for './resources/workers'. Nice cleanup.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. lib/resources/destinations.rb:17
  • Draft comment:
    Removed the redundant 'ACTION: still needs to be tested' comment from the match_metadata method. Ensure corresponding tests are in place.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. lib/resources/organizations.rb:17
  • Draft comment:
    Removed redundant testing comment from the insert_task method. Ensure that tests are implemented for this endpoint.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
4. lib/resources/recipients.rb:39
  • Draft comment:
    Removed redundant testing comment from the match_metadata method. Make sure tests cover this functionality.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
5. lib/resources/routeplans.rb:38
  • Draft comment:
    Removed extraneous whitespace in the add_tasks_to_routeplan method signature.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
6. lib/resources/tasks.rb:15
  • Draft comment:
    Removed multiple redundant 'ACTION: still needs to be tested' comments from Tasks methods. Confirm that proper tests exist for batch creation, async creation, complete, auto_assign, and match_metadata.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
7. lib/resources/webhooks.rb:10
  • Draft comment:
    Removed redundant testing comments from the Webhooks methods. Ensure appropriate tests are in place to verify webhook functionality.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
8. lib/resources/workers.rb:51
  • Draft comment:
    Updated action comments to indicate that get_by_location, insert_task, and match_metadata have been tested. This clarifies their implementation status.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_PWVC5nhABuWGhAjy

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

if query_parameters_hash
query_parameters = URI.encode_www_form(query_parameters_hash)
end
path = "teams/#{id}/tasks?#{query_parameters}"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In get_unassigned_tasks, if no query parameters are provided, the URL will end with '?nil'. Consider conditionally appending the query string only when query parameters exist. For example:

path = "teams/#{id}/tasks"
path += "?#{URI.encode_www_form(query_parameters_hash)}" if query_parameters_hash

Suggested change
path = "teams/#{id}/tasks?#{query_parameters}"
path = "teams/#{id}/tasks" + (query_parameters ? "?#{query_parameters}" : "")

This comment was generated because it violated a code review rule: mrule_MmFUQCGeCc2tGBoo.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Looks good to me! 👍

Reviewed 45409ed in 47 seconds. Click for details.
  • Reviewed 28 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 draft comments. View those below.
  • Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. lib/resources/workers.rb:52
  • Draft comment:
    Removed an outdated comment about testing. Ensure that proper tests exist instead of relying on inline TODOs.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
2. lib/resources/workers.rb:90
  • Draft comment:
    Removed an outdated testing reminder comment before insert_task. Verify that tests cover this endpoint.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None
3. lib/resources/workers.rb:97
  • Draft comment:
    Removed an outdated testing reminder comment before match_metadata. Consider adding verification tests if missing.
  • Reason this comment was not posted:
    Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_Z1xKaeYJnjD9Ijkl

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant