Beacon Configuration Admin View, with CRUD and Regenerate and Revoke Tokens Actions #585
Conversation
…-api-keys - Removed old beacon migration that conflicted with device migrations - Updated BeaconsController to use Beacons::Creator service for API key generation - Changed beacon relationships from direct provider/tags to many-to-many beacon_providers and beacon_topics - Updated forms to use provider_ids and topic_ids multi-selects instead of single provider_id and tag_ids - Updated show page to display API key (when just created) instead of token - Updated index page to show region, language, provider count, and topic count - Removed beacon_tag model and references as we now use beacon_topics - Changed status from online/offline to active/revoked based on revoked_at timestamp
Topics use 'title' column, not 'name'. Updated controller and views to use Topic.active.order(:title)
- Update API key display messaging for admin context - Add regenerate_key action in BeaconsController using KeyRegenerator service - Add 'Regenerate API Key' button on show page with confirmation dialog - Full API key still shown only after creation/regeneration via flash - Clearer messaging that full key is displayed immediately after provisioning
- Added document_count method to Beacon model to count available topics - Added file_count method to count actual attached document files - Updated show page with 4 stat cards: Status, Available Topics, Document Files, and Configured Filters - Logic respects beacon's language, providers (or region if no providers), and topic filters
Resolves #572 Adds actions to revoke and regenerate tokens. Refactors the previous "regenerate" code in the controller to be in a model TODO: - Controller specs for beacons controller - Route for revoking a Token
FionaLMcLaren
changed the title
FionaLMcLaren
changed the title
spec/requests/beacons_spec.rb
Outdated
| context "when there is an error" do | ||
| allow(Beacon).to receive(:regenerate).raise_error | ||
|
|
||
| it "does not regenerate the API key for the requested beacon" do |
There was a problem hiding this comment.
These specs seem rather redundant, as the logic is already tested for in the model spec. I wasn't sure if to keep going with these tests for POST /revoke_key, or to keep it as testing that the redirects work? I used the code from our other request specs are reference
There was a problem hiding this comment.
The question here is: Do I bother with these specs, if we have the logic verified by the model spec; or do we want to test that revoking/regenerating a token works in these request specs?
There was a problem hiding this comment.
Decided it was ok to get rid of redundant tests in request spec 👍
There was a problem hiding this comment.
We can test individual methods within models (unit specs) and the whole flow with requests specs.
Since request spec is more about integration testing, please prefer requests specs over model specs if you need to choose
FionaLMcLaren
force-pushed
the
beacons-regenerate
branch
from
dbac880 to
ca83796
Compare
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Add libffi-dev package to Dockerfile.dev for Alpine Linux to support ffi gem (dependency).
Allows Bundler to resolve dependencies for Darwin 25.
Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Sean Marcia <seanmarcia@github.com>
FionaLMcLaren
force-pushed
the
beacons-regenerate
branch
from
2071a2e to
3954d6e
Compare
|
Hey @FionaLMcLaren for a ticket with so many visual elements, it's really helpful to put some screenshots in the PR description, particularly for:
Thanks! Solid work! |
devjona
left a comment
devjona
left a comment
There was a problem hiding this comment.
Solid work! I left a few comments.
dmitrytrager
left a comment
dmitrytrager
left a comment
There was a problem hiding this comment.
have some comments, mostly about code validation and data integrity
| @@ -0,0 +1,94 @@ | |||
| class BeaconsController < ApplicationController | |||
| before_action :redirect_contributors | |||
There was a problem hiding this comment.
Suggestion: add base beacons controller and move this before action there
| flash[:notice] = "API key has been successfully revoked." | ||
| redirect_to @beacon | ||
|
|
||
| rescue => e |
There was a problem hiding this comment.
You can use rescue without begin/end
There was a problem hiding this comment.
For this case (since we control revoke process) could be better to return result from revoke method and just have simple condition here
The same comment for regeneration
There was a problem hiding this comment.
Hey! Thank you for your review - it's been very helpful :) Do you have an idea how we should then use the result from the revoke method when we have an error? Cheers!
There was a problem hiding this comment.
You can catch error there and return false
There was a problem hiding this comment.
I think we can skip it, simple update rarely returns errors
| # It's strongly recommended that you check this file into your version control system. | ||
|
|
||
| ActiveRecord::Schema[8.1].define(version: 2026_02_03_100004) do | ||
| ActiveRecord::Schema[8.1].define(version: 2026_02_03_100134) do |
There was a problem hiding this comment.
Not sure why did schema change. Were there any migrations?
There was a problem hiding this comment.
This happened because of a merge conflict resolution - What version number is expected here?
There was a problem hiding this comment.
What was the reason for conflict? Do we update version here in this branch?
FionaLMcLaren
force-pushed
the
beacons-regenerate
branch
2 times, most recently
from
9931cda to
a628a77
Compare
FionaLMcLaren
force-pushed
the
beacons-regenerate
branch
from
a628a77 to
a24368c
Compare
6 participants
What Issue Does This PR Cover, If Any?
Resolves #572
What Changed? And Why Did It Change?
This PR adds the actions to revoke and regenerate tokens. It refactors the previous
"regenerate" code in the controller to be in a model. These actions are accessible to an admin user on their Admin Dashboard, when viewing a Beacon. This PR also does some cleanup on the views here, e.g.: removing the flashes for
:api_key, as the information is already included in the success flashHow Has This Been Tested?
RSpec
Please Provide Screenshots