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.

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

Add button_to execute :read task #26

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

hmdros
Copy link
Contributor

@hmdros hmdros commented Oct 3, 2023

Attending to issue #22, this PR aims to add a button on navigation bar to trigger :read task and sync Sitemap links.

@hmdros hmdros linked an issue Oct 3, 2023 that may be closed by this pull request
@hmdros hmdros requested a review from fbuys October 3, 2023 13:23
def execute_read_task
system('rake read')
redirect_to root_path
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not quite sure which controller is best for this action, but we don't have much information at this stage - so this should be fine.

@@ -15,6 +15,10 @@
<a class="inline-block border rounded py-1 px-3" href="/?domain=UpgradeJS.com">UpgradeJS Links</a>
</li>

<li class="mr-3">
<%= button_to "Execute 'Read' task", execute_read_task_path, class: "inline-block border rounded py-1 px-3", method: :post %>
Copy link
Contributor

Choose a reason for hiding this comment

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

How about we use the disable_with data option to disable the button after it is clicked?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

disable_with won't work as expected, because the redirect will reload the page.

@FionaDL FionaDL temporarily deployed to librarian-22-add-a-butt-jhf8p8 October 3, 2023 20:11 Inactive
@FionaDL FionaDL temporarily deployed to librarian-staging October 3, 2023 20:11 Inactive
def execute_read_task
system('rake read')
redirect_back_or_to root_path
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be the last line of the method?

@@ -15,6 +15,10 @@
<a class="inline-block border rounded py-1 px-3" href="/?domain=UpgradeJS.com">UpgradeJS Links</a>
</li>

<li class="mr-3">
<%= button_to "Run 'Sitemap' task", execute_read_task_path, class: "inline-block border rounded py-1 px-3", method: :post %>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<%= button_to "Run 'Sitemap' task", execute_read_task_path, class: "inline-block border rounded py-1 px-3", method: :post %>
<%= button_to "Refresh List", execute_read_task_path, class: "inline-block border rounded py-1 px-3", method: :post %>

Copy link
Member

@FionaDL FionaDL left a comment

Choose a reason for hiding this comment

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

@hmdros looks good, QA passed, but let's change the button title if you agree(I left a suggestion). Other then that, great job!

Using redirect_back_or_to to redirect after read task completed
successfully.

See: #22
@fbuys fbuys force-pushed the 22-add-a-button-to-run-the-sitemap-reader-job branch from dead939 to 6743fbf Compare October 4, 2023 14:59
@fbuys fbuys merged commit 43d91e8 into main Oct 4, 2023
1 check passed
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.

Add a button to run the sitemap reader job
3 participants