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

Migrate Create newcomers entry page to react #10732

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions app/controllers/admin_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -418,22 +418,17 @@ def do_anonymize_person
render 'anonymize_person'
end

def finish_unfinished_persons
@finish_persons = FinishPersonsForm.new(
competition_ids: params[:competition_ids] || nil,
)
private def competition_list_from_string(competition_ids_string)
competition_ids_string.split(',').uniq.compact
end

def complete_persons
action_params = params.require(:finish_persons_form)
.permit(:competition_ids)

@finish_persons = FinishPersonsForm.new(action_params)
@persons_to_finish = @finish_persons.search_persons
@competition_ids = competition_list_from_string(params.fetch(:competition_ids, ""))
@persons_to_finish = FinishUnfinishedPersons.search_persons(@competition_ids)

if @persons_to_finish.empty?
flash[:warning] = "There are no persons to complete for the selected competition"
redirect_to action: :finish_unfinished_persons
redirect_to panel_page_path(id: User.panel_pages[:createNewComers], competition_ids: @competition_ids)
end
end

Expand Down Expand Up @@ -488,15 +483,14 @@ def do_complete_persons
competition_ids = params.dig(:person_completions, :competition_ids)

if continue_batch
finish_persons = FinishPersonsForm.new(competition_ids: competition_ids)
can_continue = FinishUnfinishedPersons.unfinished_results_scope(finish_persons.competitions).any?
can_continue = FinishUnfinishedPersons.unfinished_results_scope(competition_list_from_string(competition_ids)).any?

if can_continue
return redirect_to action: :complete_persons, finish_persons_form: { competition_ids: competition_ids }
return redirect_to action: :complete_persons, competition_ids: competition_ids
end
end

redirect_to action: :finish_unfinished_persons, competition_ids: competition_ids
redirect_to panel_page_path(id: User.panel_pages[:createNewComers], competition_ids: competition_ids)
end

def peek_unfinished_results
Expand Down
15 changes: 0 additions & 15 deletions app/models/finish_persons_form.rb

This file was deleted.

2 changes: 1 addition & 1 deletion app/views/admin/_import_inbox_step.erb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
All newcomers seem to have been assigned WCA IDs.
Please <%= link_to "delete the inbox rows", competition_admin_delete_inbox_data_path(@competition, model: :inbox_person), class: 'inbox-trigger btn btn-primary', data: { jquery_method: :delete } %>.
<% else %>
Please <%= link_to "assign WCA IDs to newcomers", admin_complete_persons_path(finish_persons_form: { competition_ids: @competition.id }), target: '_blank', class: 'btn btn-primary' %>
Please <%= link_to "assign WCA IDs to newcomers", admin_complete_persons_path(competition_ids: [@competition.id]), target: '_blank', class: 'btn btn-primary' %>
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to wrap this in an array? Passing it as a string should be just fine

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it do accept single string as well I guess, but there are cases where multiple competitions are also needed to be given as input. So, to make the type of competition_ids as a single type (array of string) instead of multiple type (array of string or single string), I thought of doing this way. Is it fine?

to proceed to the next step.
<% end %>
</p>
Expand Down
10 changes: 5 additions & 5 deletions app/views/admin/complete_persons.html.erb
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
<% provide(:title, 'Finish unfinished persons') %>

<div class="container">
<% competition_ids = @finish_persons.competition_ids %>
<% competition_ids = @competition_ids %>
Copy link
Member

Choose a reason for hiding this comment

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

This line is redundant now, if you already have a @competition_ids variable then use it everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

No need to re-assign it to "itself" with another name

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, instead of changing in many places, I thought of doing this way. Changed all the occurrences.


<h2>
<%= yield(:title) %>
<%= link_to "Go back", admin_finish_unfinished_persons_path(competition_ids: competition_ids), class: "btn btn-default" %>
<%= link_to "Go back", panel_page_path(id: User.panel_pages[:createNewComers], competition_ids: competition_ids), class: "btn btn-default" %>
Copy link
Member

Choose a reason for hiding this comment

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

Does this work as expected when competition_ids is an already-split array?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for pointing that out, I actually didn't test that portion and realized that it was not even working for single competition ID as it was getting lost during redirection. I've fixed it now.

</h2>

<% if competition_ids.present? %>
<p>For competition(s):</p>

<ul>
<% @finish_persons.competitions.each do |competition_id| %>
<% competition_ids.each do |competition_id| %>
<li><code><%= competition_id %></code></li>
<% end %>
</ul>
Expand Down Expand Up @@ -42,7 +42,7 @@

<tr class="active">
<td colspan="8" class="text-center">
<% if competition_ids.present? && @finish_persons.competitions.length == 1 %>
<% if competition_ids.present? && competition_ids.length == 1 %>
<b><%= p[:person_name] %></b> (ID <code><%= p[:person_id] %></code>)
<% else %>
<b><%= p[:person_name] %></b> (ID <code><%= p[:person_id] %></code>, Competition <code><%= p[:competition_id] %></code>)
Expand Down Expand Up @@ -116,7 +116,7 @@
<% end %>
<%= f.input :continue_batch, as: :boolean, label: "Continue with the next batch of newcomers if there are more pending entries", hint: "If you are redirected to the 'Create Newcomers' entry point, it means that there are no more pending entries.", input_html: { checked: competition_ids.present? } %>
<%= f.button :submit, value: "Submit changes!", class: "btn-primary" %>
<%= link_to "Go back", admin_finish_unfinished_persons_path(competition_ids: competition_ids), class: "btn btn-default" %>
<%= link_to "Go back", panel_page_path(id: User.panel_pages[:createNewComers], competition_ids: competition_ids), class: "btn btn-default" %>
<% end %>
</div>

Expand Down
35 changes: 0 additions & 35 deletions app/views/admin/finish_unfinished_persons.html.erb

This file was deleted.

6 changes: 3 additions & 3 deletions app/webpacker/components/Panel/PanelPages.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
generateDbTokenUrl,
serverStatusPageUrl,
runValidatorsUrl,
createNewComersUrl,
checkRecordsUrl,
computeAuxiliaryDataUrl,
generateDataExportsUrl,
Expand Down Expand Up @@ -39,6 +38,7 @@ import DownloadVoters from './pages/DownloadVoters';
import ApprovePictures from './pages/ApprovePictures';
import EditPersonRequestsPage from './pages/EditPersonRequestsPage';
import AnonymizationScriptPage from './pages/AnonymizationScriptPage';
import CreateNewcomersPage from './pages/CreateNewcomersPage';

const DELEGATE_HANDBOOK_LINK = 'https://documents.worldcubeassociation.org/edudoc/delegate-handbook/delegate-handbook.pdf';

Expand Down Expand Up @@ -168,8 +168,8 @@ export default {
link: runValidatorsUrl,
},
[PANEL_PAGES.createNewComers]: {
name: 'Create New Comers',
link: createNewComersUrl,
name: 'Create Newcomers',
component: CreateNewcomersPage,
},
[PANEL_PAGES.checkRecords]: {
name: 'Check Records',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import React from 'react';
import {
Button, Form, Header, HeaderSubheader,
} from 'semantic-ui-react';
import { IdWcaSearch } from '../../../SearchWidget/WcaSearch';
import SEARCH_MODELS from '../../../SearchWidget/SearchModel';
import { viewUrls } from '../../../../lib/requests/routes.js.erb';
import useQueryParams from '../../../../lib/hooks/useQueryParams';
import useInputState from '../../../../lib/hooks/useInputState';

export default function CompetitionsInput() {
const [queryParams] = useQueryParams();
const competitionIdsFromQuery = queryParams?.competition_ids?.split(',');
const [competitionIds, setCompetitionIds] = useInputState(competitionIdsFromQuery || []);

return (
<Form>
<IdWcaSearch
model={SEARCH_MODELS.competition}
multiple
value={competitionIds}
onChange={setCompetitionIds}
label="Competition(s)"
/>
<Header as="h4">
<HeaderSubheader>
Leave blank to check for all competitions
</HeaderSubheader>
</Header>
<Button
primary
size="big"
href={viewUrls.admin.completePersons(competitionIds.length > 0 ? competitionIds : null)}
>
Check newcomers
</Button>
</Form>
);
}
47 changes: 47 additions & 0 deletions app/webpacker/components/Panel/pages/CreateNewcomersPage/index.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import React from 'react';
import { Header, List, ListItem } from 'semantic-ui-react';
import CompetitionsInput from './CompetitionsInput';

export default function CreateNewcomersPage() {
return (
<>
<Header>Create Newcomers</Header>
<Information />
<CompetitionsInput />
</>
);
}

function Information() {
return (
<>
<p>
In this script, a &quot;person&quot; always means a triple of (id,name,countryId) and
&quot;similar&quot; always means just name similarity. A person is called
&quot;finished&quot; if it has a non-empty personId. A &quot;semi-id&quot; is the id
without the running number at the end.
</p>
<p>
For each unfinished person in the Results table, I show you the few most similar
persons. Then you make choices and click &quot;update&quot; at the bottom of the page
to show and execute your choices. You can:
</p>
<List bulleted>
<ListItem>
Choose the person as &quot;new&quot;, optionally modifying name, country and
semi-id. This will add the person to the Persons table (with appropriately
extended id) and change its Results accordingly. If this person has both roman and
local names, the syntax for the names to be inserted correctly is
&quot;romanName (localName)&quot;.
</ListItem>
<ListItem>
Choose another person. This will overwrite the person&apos;s (id,name,countryId)
triple in the Results table with those of the other person.
</ListItem>
<ListItem>
Skip it if you&apos;re not sure yet.
</ListItem>
</List>
</>
);
}
7 changes: 4 additions & 3 deletions app/webpacker/lib/requests/routes.js.erb
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,10 @@ export const viewUrls = {
tickets: {
list: (type, status, sort) => `<%= CGI.unescape(Rails.application.routes.url_helpers.tickets_path) %>?${jsonToQueryString({ type, status, sort })}`,
show: (ticketId) => `<%= CGI.unescape(Rails.application.routes.url_helpers.ticket_path("${ticketId}")) %>`,
}
},
admin: {
completePersons: (competitionIds) => `<%= CGI.unescape(Rails.application.routes.url_helpers.admin_complete_persons_path) %>?${jsonToQueryString({ competition_ids: competitionIds })}`,
},
}

export const actionUrls = {
Expand Down Expand Up @@ -312,8 +315,6 @@ export const serverStatusPageUrl = `<%= CGI.unescape(Rails.application.routes.ur

export const runValidatorsUrl = `<%= CGI.unescape(Rails.application.routes.url_helpers.admin_check_results_path) %>`;

export const createNewComersUrl = `<%= CGI.unescape(Rails.application.routes.url_helpers.admin_finish_unfinished_persons_path) %>`;

export const checkRecordsUrl = `<%= CGI.unescape(Rails.application.routes.url_helpers.admin_check_regional_records_path) %>`;

export const computeAuxiliaryDataUrl = `<%= CGI.unescape(Rails.application.routes.url_helpers.admin_compute_auxiliary_data_path) %>`;
Expand Down
3 changes: 0 additions & 3 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@
get 'panel/:panel_id' => 'panel#index', as: :panel_index
scope 'panel-page' do
get 'run-validators' => 'admin#check_results', as: :admin_check_results
get 'create-new-comers' => 'admin#finish_unfinished_persons', as: :admin_finish_unfinished_persons
get 'check-records' => 'admin#check_regional_records', as: :admin_check_regional_records
get 'compute-auxiliary-data' => 'admin#compute_auxiliary_data', as: :admin_compute_auxiliary_data
get 'generate-data-exports' => 'admin#generate_exports', as: :admin_generate_exports
Expand Down Expand Up @@ -291,8 +290,6 @@
get '/admin/do_generate_public_export' => 'admin#do_generate_public_export'
get '/admin/override_regional_records' => 'admin#override_regional_records'
post '/admin/override_regional_records' => 'admin#do_override_regional_records'
get '/admin/finish_persons' => 'admin#finish_persons'
post '/admin/finish_persons' => 'admin#do_finish_persons'
get '/admin/complete_persons' => 'admin#complete_persons'
post '/admin/complete_persons' => 'admin#do_complete_persons'
get '/admin/peek_unfinished_results' => 'admin#peek_unfinished_results'
Expand Down