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

Refactor admin poll section #4990

Merged
merged 19 commits into from
Sep 14, 2022
Merged

Refactor admin poll section #4990

merged 19 commits into from
Sep 14, 2022

Conversation

taitus
Copy link
Member

@taitus taitus commented Sep 12, 2022

References

Objectives

  • Remove unused code
  • Use load_and_authorize_resource whenever possible
  • Remove the ambiguous hidden fields from some forms.
  • Improve the navigation in the polls section with better redirects and back links

Notes

Maybe we can improve the action :documents from Admin::Poll::Questions::AnswersController.
Right now the load_and_authorize_resource :answer include a "except: :documents" and @answer is loaded in :documents action in commit Load answer through question in answers controller

Actions :search_booths and :search_officers in admin polls controller
are moved to other controllers since commit 20e3113 for
:search_booths and commit 19ec7f9 for :search_officers.

This then allows us to remove the code that references these actions in
the controller and in the administrator abilities.
Since commit adf18ee  this action no longer makes sense.
@javierm javierm added this to Reviewing in Consul Democracy Sep 12, 2022
@javierm javierm removed the Polls label Sep 13, 2022
@javierm javierm self-assigned this Sep 13, 2022
@javierm javierm added the UX label Sep 13, 2022
After removing a question from a poll it makes more sense to redirect to
your own poll show page in order to manage their questions.

Currently it is redirecting to the questions index page where all the
questions from all the polls are displayed and takes you completely out
of the context of the poll you are in.

In the future we will remove this index question page.
@taitus taitus force-pushed the improve-polls branch 3 times, most recently from 241f090 to 28d944b Compare September 14, 2022 10:13
taitus and others added 11 commits September 14, 2022 14:45
We are simplifying the load answer and we can remove the ambiguous
hidden field from answer form.
This way we have a controller just to manage
Poll::Question::Answer related documents in the
same way we have for videos and images.
We might change the style of this caption or remove it completely in the
future. In the meantime, we use the right HTML tag for the task to give
information regarding what the table is about, and that tag is
`<caption>`.
Until now, in order to edit an answer, we had to click on its title on
the table and then on the "Edit answer" link.

That was tedious and different from what we usually do in the admin
section. Furthermore, the code for the answers table was written twice
and when we modified it we forgot to update the one in the `show`
action, meaning the table here provided less information than the
information present in the answers tables.

Co-Authored-By: Javi Martín <javim@elretirao.net>
These methods aren't necessary since commit 71601bd.
Minor change for replace @video.answer_id to @video.answer.
These routes aren't used since commits (2993ef8, 88a7a29)
In parallel while these routes were being removed, the route file
was being split into different files, and they were included again
in the commit 1cd47da.
In some cases (e.g. after editing or creating a resource
with errors) the default back_link did not redirect to the
expected page.

Now we force the back_links to the index pages, so we
always get the desired redirect.
Consul Democracy automation moved this from Reviewing to Testing Sep 14, 2022
@taitus taitus merged commit eaafc7a into master Sep 14, 2022
Consul Democracy automation moved this from Testing to Release 1.6.0 Sep 14, 2022
@taitus taitus deleted the improve-polls branch September 14, 2022 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants