Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sweep: Pagination Missing on "Shop Summary" Merchant Table (✓ Sandbox Passed) #99

Closed
wants to merge 20 commits into from
Closed

Sweep: Pagination Missing on "Shop Summary" Merchant Table (✓ Sandbox Passed) #99

wants to merge 20 commits into from

Conversation

sweep-ai[bot]
Copy link

@sweep-ai sweep-ai bot commented Feb 3, 2024 •
edited
Loading

PR Feedback: 👍

Description

This pull request adds pagination functionality to the "Shop Summary" page in the application. It includes changes to the Summarys::ShopController, app/helpers/pagination_helper.rb, app/models/summary/shop.rb, and app/views/summarys/index.html.erb files.

Summary

  • Added @page and @per_page variables to the index action in Summarys::ShopController
  • Modified the summarize method in Summary::Shop to accept page and per_page parameters
  • Added the PaginationHelper module to the pagination_helper.rb file
  • Modified the summarized_shop_data method in Summary::Shop to include pagination logic
  • Added pagination HTML to the index.html.erb file

Fixes #96.

🎉 Latest improvements to Sweep:
  • New dashboard launched for real-time tracking of Sweep issues, covering all stages from search to coding.
  • Integration of OpenAI's latest Assistant API for more efficient and reliable code planning and editing, improving speed by 3x.
  • Use the GitHub issues extension for creating Sweep issues directly from your editor.

💡 To get Sweep to edit this pull request, you can:

  • Comment below, and Sweep can edit the entire PR
  • Comment on a file, Sweep will only modify the commented file
  • Edit the original issue to get Sweep to recreate the PR from scratch

@sweep-ai Sweep AI
Copy link
Author

sweep-ai bot commented Feb 3, 2024 •
edited
Loading

Rollback Files For Sweep

  • Rollback changes to app/controllers/summarys/shop_controller.rb
  • Rollback changes to app/models/summary/shop.rb
  • Rollback changes to app/views/summarys/index.html.erb

@sweep-ai sweep-ai bot added the sweep label Feb 3, 2024
@sweep-ai sweep-ai bot mentioned this pull request Feb 3, 2024
Open
8 tasks
forsbergplustwo
forsbergplustwo reviewed Feb 3, 2024
View reviewed changes
app/helpers/pagination_helper.rb Outdated Show resolved Hide resolved
app/helpers/pagination_helper.rb Outdated Show resolved Hide resolved
app/helpers/pagination_helper.rb Outdated Show resolved Hide resolved
app/helpers/pagination_helper.rb Outdated Show resolved Hide resolved
app/helpers/pagination_helper.rb Outdated Show resolved Hide resolved
app/helpers/pagination_helper.rb Outdated Show resolved Hide resolved
app/helpers/pagination_helper.rb Outdated Show resolved Hide resolved
@forsbergplustwo
Copy link
Owner

  1. Format using standardrb gem
  2. Remove ability for user to change per_page number via params, use constant instead.
  3. Use polaris_pagination() view helper from Polaris View Components gem instead of custom pagination_helper. Polaris pagination helper provides the following:
# frozen_string_literal: true

module Polaris
  class PaginationComponent < Polaris::Component
    def initialize(
      previous_url: nil,
      next_url: nil,
      label: nil,
      **system_arguments
    )
      @previous_url = previous_url
      @next_url = next_url
      @label = label

      @system_arguments = system_arguments
      @system_arguments["aria-label"] = "Pagination"

      @button_group_arguments = {}
      @button_group_arguments[:segmented] = !label.present?
    end
  end
end

@sweep-ai Sweep AI
Copy link
Author

sweep-ai bot commented Feb 3, 2024 •
edited
Loading

🚀 Wrote Changes

Done.

This is an automated message generated by Sweep AI.

forsbergplustwo
forsbergplustwo requested changes Feb 3, 2024
View reviewed changes
Copy link
Owner

@forsbergplustwo forsbergplustwo left a comment

Choose a reason for hiding this comment

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

Good, see comment and make sure all changed lines of code use double-quoted strings 👍

app/helpers/pagination_helper.rb Outdated Show resolved Hide resolved
@forsbergplustwo
Copy link
Owner

Remove the pagination helper file, it’s no longer needed. Update pr to use double-quoted strings only.

@sweep-ai Sweep AI
Copy link
Author

sweep-ai bot commented Feb 3, 2024 •
edited
Loading

❌ Failed to make changes

Please join our Discord to report this issue.

This is an automated message generated by Sweep AI.

@forsbergplustwo
Copy link
Owner

Great, fix any bugs related to first and last page of results (don’t include the url to the pagination view helper).

@sweep-ai Sweep AI
Copy link
Author

sweep-ai bot commented Feb 3, 2024 •
edited
Loading

🚀 Wrote Changes

Done.

This is an automated message generated by Sweep AI.

@forsbergplustwo
Copy link
Owner

Add a controller test for this, and make sure pagination works. You may need to set per_page to 1 in the test temporarily, so it works.

@sweep-ai Sweep AI
Copy link
Author

sweep-ai bot commented Feb 4, 2024 •
edited
Loading

🚀 Wrote Changes

Done.

This is an automated message generated by Sweep AI.

forsbergplustwo
forsbergplustwo reviewed Feb 4, 2024
View reviewed changes
test/controllers/summarys/shop_controller_test.rb Outdated
end

test "index pagination" do
get :index, params: { selected_app: @selected_app.id, page: 1 }
Copy link
Owner

Choose a reason for hiding this comment

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

[standard] <Layout/SpaceInsideHashLiteralBraces> reported by reviewdog 🐶
Space inside { detected.

Suggested change
get :index, params: { selected_app: @selected_app.id, page: 1 }
get :index, params: {selected_app: @selected_app.id, page: 1}

Copy link
Author

@sweep-ai sweep-ai bot Feb 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

🚀 Wrote Changes

Done.

This is an automated message generated by Sweep AI.

test/controllers/summarys/shop_controller_test.rb Outdated
end

test "index pagination" do
get :index, params: { selected_app: @selected_app.id, page: 1 }
Copy link
Owner

Choose a reason for hiding this comment

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

[standard] <Layout/SpaceInsideHashLiteralBraces> reported by reviewdog 🐶
Space inside } detected.

Suggested change
get :index, params: { selected_app: @selected_app.id, page: 1 }
get :index, params: { selected_app: @selected_app.id, page: 1

Copy link
Author

@sweep-ai sweep-ai bot Feb 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

🚀 Wrote Changes

Done.

This is an automated message generated by Sweep AI.

forsbergplustwo
forsbergplustwo reviewed Feb 4, 2024
View reviewed changes
test/controllers/summarys/shop_controller_test.rb Outdated
end

test "index pagination" do
get :index, params: { selected_app: @selected_app.id, page: 1}
Copy link
Owner

Choose a reason for hiding this comment

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

[standard] <Layout/SpaceInsideHashLiteralBraces> reported by reviewdog 🐶
Space inside { detected.

Suggested change
get :index, params: { selected_app: @selected_app.id, page: 1}
get :index, params: {selected_app: @selected_app.id, page: 1}

Copy link
Author

@sweep-ai sweep-ai bot Feb 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

🚀 Wrote Changes

Done.

This is an automated message generated by Sweep AI.

forsbergplustwo
forsbergplustwo reviewed Feb 4, 2024
View reviewed changes
test/controllers/summarys/shop_controller_test.rb Outdated
end

test "index pagination" do
get :index, params: {selected_app: @selected_app.id, page: 1 }
Copy link
Owner

Choose a reason for hiding this comment

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

[standard] <Layout/SpaceInsideHashLiteralBraces> reported by reviewdog 🐶
Space inside } detected.

Suggested change
get :index, params: {selected_app: @selected_app.id, page: 1 }
get :index, params: {selected_app: @selected_app.id, page: 1

Copy link
Author

@sweep-ai sweep-ai bot Feb 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

🚀 Wrote Changes

I wasn't able to make changes. This could be due to an unclear request or a bug in my code.
As a reminder, comments on a file only modify that file. Comments on a PR (at the bottom of the "conversation" tab) can modify the entire PR. Please try again or contact us on Discord

This is an automated message generated by Sweep AI.

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

@forsbergplustwo forsbergplustwo forsbergplustwo requested changes

Assignees
No one assigned
Labels
sweep
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pagination Missing on "Shop Summary" Merchant Table
1 participant
@forsbergplustwo