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

Make admin controllers restful #836

Merged
merged 8 commits into from
Jan 26, 2021
Merged

Conversation

solebared
Copy link
Collaborator

@solebared solebared commented Jan 22, 2021

Why

AdminController currently contains a handful of non-restful actions. In addition to their non-restfulness, they also make it hard to inherit from AdminController (see related discussion in #835).

Pre-Merge Checklist

  • All new features have been described in the pull request
  • Security & accessibility have been considered
  • High quality tests have been added, or an explanation has been given why the features cannot be tested
  • New features have been documented, and the code is understandable and well commented
  • Entry added to CHANGELOG.md if appropriate
  • All outstanding questions and concerns have been resolved
  • Any next steps that seem like good ideas have been created as issues for future discussion & implementation

What

When reviewing, probably easiest to look at each commit separately.

  • Extract the following restful controllers:
  • Remove unused /admin/forms page (1fb7777)
  • Move edit/update action into existing GlossaryController (e5ae91f)
  • Secure GlossaryController
    • Authorize with pundit so that edit/update aren't publicly accessible
    • Hide 'Edit Glossary' link on show page based on auth
  • Seed glossary: now that glossary admin is correctly integrated with /show, the previously hardcoded glossary will be lost. It will need to be moved into a seed file or similar. Extracted as Add seed for glossary #842

Next Steps

@svileshina is taking over this branch to finish out the remaining steps ^^. 🙌🏾

Outstanding Questions, Concerns and Other Notes

?

Security

Will need to ensure /glossary is correctly secured. All other extracted resources remain inaccessible w/o login.

Replaces AdminController#landing_page.
This page links to Forms, FormQuestions and CustomFormQuestions, all of
which are accessible from the admin dashboard. It also doesn't seem to
be linked from anywhere.
... into GlossaryController, and consolidate views.

Note two important loose ends:
1. The edit/update actions need to be secured with pundit.
2. The current glossary was hardcoded in /glossary/show.html.erb. Since
this commit changes that to be dynamic, we'll want to put the static
contents into a seed file or similar.
@svileshina svileshina force-pushed the make-admin-controllers-restful branch from 0b48cca to 083b6ff Compare January 26, 2021 19:34
Conflicts:
  - app/controllers/admin_controller.rb
@solebared solebared marked this pull request as ready for review January 26, 2021 23:14
@solebared solebared merged commit b730ab0 into main Jan 26, 2021
@solebared solebared deleted the make-admin-controllers-restful branch January 26, 2021 23:14
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.

3 participants