fix(UI): Minor enchancements and sercets related fixes #854
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review changes with Changed Files
|
WalkthroughFrontend updates include adding Clipboard web-sys feature support, refactoring key rotation modals (renaming KeyRotationModal to WorkspaceKeyRotationModal, adding MasterKeyRotationModal), updating PortalModal component signature with optional heading, enhancing webhook form instructions, and refactoring state management across organisations, workspace, secrets list, and webhooks pages. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ayushjain17
added
bug
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@crates/frontend/src/pages/organisations.rs`:
- Around line 180-276: The clipboard write in the copy_to_clipboard closure
currently ignores that write_text returns a JS Promise; replace the sync call
with proper promise handling by calling
window.navigator().clipboard().write_text(&key), wrapping that Promise with
wasm_bindgen_futures::JsFuture::from(...) and driving it with
wasm_bindgen_futures::spawn_local so you can await it and handle errors; on
success call enqueue_alert("Key copied to clipboard", AlertType::Success, 3000)
and on failure enqueue an error alert (and optionally log the JS error). Update
the copy_to_clipboard closure in the popup_state handling (the closure that
references key_to_copy) to use spawn_local+JsFuture and import
wasm_bindgen_futures, leaving generate_master_key and key_generation_rws usage
unchanged.
🧹 Nitpick comments (2)
crates/frontend/src/components/webhook_form.rs (1)
266-266: Pre-existing typo: "Paylaod" should be "Payload".While not part of this PR's changes, there's a typo in the label title. Consider fixing it while you're in this file.
📝 Suggested fix
- <Label title="Paylaod Version" /> + <Label title="Payload Version" />crates/frontend/src/pages/secrets_list.rs (1)
64-78: Consider extracting sort callback logic into a helper.The sort callback logic at lines 64-78, 95-109, and 126-140 is nearly identical, differing only in the
SortOnvariant. A helper function could reduce duplication:fn create_sort_callback( sort_on: SortOn, filters_rws: RwSignal<SecretFilters>, pagination_params_rws: RwSignal<PaginationParams>, ) -> Callback<()> { Callback::new(move |_| { let filters = filters_rws.get(); let current_sort_on = filters.sort_on.unwrap_or_default(); let sort_by = if current_sort_on == sort_on { filters.sort_by.unwrap_or_default().flip() } else { SortBy::Desc }; let new_filters = SecretFilters { sort_on: Some(sort_on), sort_by: Some(sort_by), ..filters }; pagination_params_rws.update(|f| f.reset_page()); filters_rws.set(new_filters); }) }This is optional and can be deferred if you prefer keeping the current explicit approach.
Comment on lines
+180
to
+276
| {move || match key_generation_rws.get() { | ||
| Some(popup_state) => { | ||
| view! { | ||
| <PortalModal | ||
| heading="Generate Master Encryption Key" | ||
| handle_close=move |_| key_generation_rws.set(None) | ||
| > | ||
| <div class="alert alert-warning"> | ||
| <i class="ri-alert-line" /> | ||
| <span> | ||
| "This will generate a new master encryption key. Save it securely - it will not be shown again." | ||
| </span> | ||
| </div> | ||
| {match popup_state.clone() { | ||
| Some(generated_key) => { | ||
| let key_to_copy = generated_key.clone(); | ||
| let copy_to_clipboard = move |_| { | ||
| let key = key_to_copy.clone(); | ||
| if let Some(window) = web_sys::window() { | ||
| let _ = window.navigator().clipboard().write_text(&key); | ||
| enqueue_alert( | ||
| "Key copied to clipboard".to_string(), | ||
| AlertType::Success, | ||
| 3000, | ||
| ); | ||
| } | ||
| }; | ||
|
|
||
| {move || { | ||
| if let Some(generated_key) = generated_key.get() { | ||
| view! { | ||
| <> | ||
| <div class="bg-gray-900 text-green-400 p-4 rounded-lg font-mono text-sm mt-2 break-all select-all"> | ||
| {generated_key} | ||
| view! { | ||
| <div class="flex flex-col gap-2"> | ||
| <div class="control-form"> | ||
| <Label title="Generated Key" /> | ||
| <div class="p-4 flex justify-between items-center bg-gray-900 text-green-400 rounded-lg"> | ||
| <div class="font-mono text-[14px] break-all select-all"> | ||
| {generated_key} | ||
| </div> | ||
| <button | ||
| type="button" | ||
| class="px-1 bg-gray-800 hover:bg-gray-700 text-white rounded transition-colors" | ||
| on:click=copy_to_clipboard | ||
| title="Copy to clipboard" | ||
| > | ||
| <i class="ri-file-copy-line" /> | ||
| </button> | ||
| </div> | ||
| <pre class="text-sm mt-2 whitespace-pre-wrap bg-base-200 p-3 rounded"> | ||
| {"1. Copy this key immediately - it will NOT be shown again.\n2. Store it securely in your secrets manager.\n3. Set it as environment variable: MASTER_ENCRYPTION_KEY=<key>\n4. Restart the service for the key to take effect."} | ||
| </pre> | ||
| <Button | ||
| text="Close" | ||
| on_click=move |_| { | ||
| set_show_master_key_generate_modal.set(false); | ||
| set_generated_key.set(None); | ||
| } | ||
| icon_class="ri-close-line" | ||
| class="mt-4 w-full" | ||
| /> | ||
| </> | ||
| } | ||
| } else { | ||
| view! { | ||
| <> | ||
| <button | ||
| class="btn btn-primary w-full" | ||
| disabled=is_generating.get() | ||
| on:click=move |_| generate_master_key_action.dispatch(()) | ||
| > | ||
| {if is_generating.get() { | ||
| "Generating..." | ||
| } else { | ||
| "Generate Key" | ||
| }} | ||
| <i class="ri-key-2-line"></i> | ||
| </button> | ||
| </> | ||
| } | ||
| </div> | ||
| <div class="control-form"> | ||
| <Label title="Instructions" /> | ||
| <ol class="p-4 flex flex-col gap-3 bg-base-200 rounded-lg list-none"> | ||
| {key_generation_instructions() | ||
| .into_iter() | ||
| .enumerate() | ||
| .map(|(index, instruction)| { | ||
| view! { | ||
| <li class="flex gap-3 items-center"> | ||
| <span class="flex-shrink-0 w-6 h-6 flex items-center justify-center bg-primary text-primary-content rounded-full text-xs font-semibold"> | ||
| {(index + 1).to_string()} | ||
| </span> | ||
| <div class="text-[14px] flex-1">{instruction}</div> | ||
| </li> | ||
| } | ||
| }) | ||
| .collect_view()} | ||
| </ol> | ||
| </div> | ||
| </div> | ||
| } | ||
| .into_view() | ||
| } | ||
| None => ().into_view(), | ||
| }} | ||
| {match popup_state.clone() { | ||
| Some(_) => { | ||
| view! { | ||
| <Button | ||
| text="Close" | ||
| on_click=move |_| key_generation_rws.set(None) | ||
| icon_class="ri-close-line" | ||
| /> | ||
| } | ||
| }} | ||
| </PortalModal> | ||
| } | ||
| }) | ||
| } | ||
| None => { | ||
| view! { | ||
| <Button | ||
| text="Generate Key" | ||
| on_click=move |_| generate_master_key() | ||
| icon_class="ri-key-2-line" | ||
| /> | ||
| } | ||
| } | ||
| }} | ||
| </PortalModal> | ||
| } | ||
| } | ||
| None => ().into_view(), | ||
| }} | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
web-sys Clipboard write_text return type promise
💡 Result:
In web-sys, web_sys::Clipboard::write_text returns a JavaScript promise:
pub fn write_text(&self, data: &str) -> js_sys::PromiseSo the return type is js_sys::Promise (not a Rust Future). You can await it from Rust by converting it with wasm_bindgen_futures::JsFuture. [1]
In the underlying Web API, Clipboard.writeText() returns a Promise that resolves once the clipboard has been updated. [2]
Sources:
[1] https://wasm-bindgen.github.io/wasm-bindgen/api/web_sys/struct.Clipboard.html
[2] https://developer.mozilla.org/en-US/docs/Web/API/Clipboard/writeText
🏁 Script executed:
# First, let's check if this codebase uses web-sys and what version
fd -e toml -e cargo | head -20
rg "web-sys" --type toml -A 2 -B 2Repository: juspay/superposition
Length of output: 5888
🏁 Script executed:
# Search for how Clipboard is used elsewhere in the codebase
rg "clipboard" -i --type rust -B 3 -A 3Repository: juspay/superposition
Length of output: 4862
Handle clipboard write failures with proper promise handling instead of ignoring failures.
The clipboard write_text() returns a js_sys::Promise, not a Rust Result, so the suggested match approach won't compile. The promise must be converted and awaited using wasm_bindgen_futures::JsFuture::from() and spawn_local() within the closure context:
🔧 Correct fix using promise handling
- let _ = window.navigator().clipboard().write_text(&key);
- enqueue_alert(
- "Key copied to clipboard".to_string(),
- AlertType::Success,
- 3000,
- );
+ let promise = window.navigator().clipboard().write_text(&key);
+ wasm_bindgen_futures::spawn_local(async move {
+ match wasm_bindgen_futures::JsFuture::from(promise).await {
+ Ok(_) => enqueue_alert(
+ "Key copied to clipboard".to_string(),
+ AlertType::Success,
+ 3000,
+ ),
+ Err(_) => enqueue_alert(
+ "Failed to copy key to clipboard".to_string(),
+ AlertType::Error,
+ 3000,
+ ),
+ }
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {move || match key_generation_rws.get() { | |
| Some(popup_state) => { | |
| view! { | |
| <PortalModal | |
| heading="Generate Master Encryption Key" | |
| handle_close=move |_| key_generation_rws.set(None) | |
| > | |
| <div class="alert alert-warning"> | |
| <i class="ri-alert-line" /> | |
| <span> | |
| "This will generate a new master encryption key. Save it securely - it will not be shown again." | |
| </span> | |
| </div> | |
| {match popup_state.clone() { | |
| Some(generated_key) => { | |
| let key_to_copy = generated_key.clone(); | |
| let copy_to_clipboard = move |_| { | |
| let key = key_to_copy.clone(); | |
| if let Some(window) = web_sys::window() { | |
| let _ = window.navigator().clipboard().write_text(&key); | |
| enqueue_alert( | |
| "Key copied to clipboard".to_string(), | |
| AlertType::Success, | |
| 3000, | |
| ); | |
| } | |
| }; | |
| {move || { | |
| if let Some(generated_key) = generated_key.get() { | |
| view! { | |
| <> | |
| <div class="bg-gray-900 text-green-400 p-4 rounded-lg font-mono text-sm mt-2 break-all select-all"> | |
| {generated_key} | |
| view! { | |
| <div class="flex flex-col gap-2"> | |
| <div class="control-form"> | |
| <Label title="Generated Key" /> | |
| <div class="p-4 flex justify-between items-center bg-gray-900 text-green-400 rounded-lg"> | |
| <div class="font-mono text-[14px] break-all select-all"> | |
| {generated_key} | |
| </div> | |
| <button | |
| type="button" | |
| class="px-1 bg-gray-800 hover:bg-gray-700 text-white rounded transition-colors" | |
| on:click=copy_to_clipboard | |
| title="Copy to clipboard" | |
| > | |
| <i class="ri-file-copy-line" /> | |
| </button> | |
| </div> | |
| <pre class="text-sm mt-2 whitespace-pre-wrap bg-base-200 p-3 rounded"> | |
| {"1. Copy this key immediately - it will NOT be shown again.\n2. Store it securely in your secrets manager.\n3. Set it as environment variable: MASTER_ENCRYPTION_KEY=<key>\n4. Restart the service for the key to take effect."} | |
| </pre> | |
| <Button | |
| text="Close" | |
| on_click=move |_| { | |
| set_show_master_key_generate_modal.set(false); | |
| set_generated_key.set(None); | |
| } | |
| icon_class="ri-close-line" | |
| class="mt-4 w-full" | |
| /> | |
| </> | |
| } | |
| } else { | |
| view! { | |
| <> | |
| <button | |
| class="btn btn-primary w-full" | |
| disabled=is_generating.get() | |
| on:click=move |_| generate_master_key_action.dispatch(()) | |
| > | |
| {if is_generating.get() { | |
| "Generating..." | |
| } else { | |
| "Generate Key" | |
| }} | |
| <i class="ri-key-2-line"></i> | |
| </button> | |
| </> | |
| } | |
| </div> | |
| <div class="control-form"> | |
| <Label title="Instructions" /> | |
| <ol class="p-4 flex flex-col gap-3 bg-base-200 rounded-lg list-none"> | |
| {key_generation_instructions() | |
| .into_iter() | |
| .enumerate() | |
| .map(|(index, instruction)| { | |
| view! { | |
| <li class="flex gap-3 items-center"> | |
| <span class="flex-shrink-0 w-6 h-6 flex items-center justify-center bg-primary text-primary-content rounded-full text-xs font-semibold"> | |
| {(index + 1).to_string()} | |
| </span> | |
| <div class="text-[14px] flex-1">{instruction}</div> | |
| </li> | |
| } | |
| }) | |
| .collect_view()} | |
| </ol> | |
| </div> | |
| </div> | |
| } | |
| .into_view() | |
| } | |
| None => ().into_view(), | |
| }} | |
| {match popup_state.clone() { | |
| Some(_) => { | |
| view! { | |
| <Button | |
| text="Close" | |
| on_click=move |_| key_generation_rws.set(None) | |
| icon_class="ri-close-line" | |
| /> | |
| } | |
| }} | |
| </PortalModal> | |
| } | |
| }) | |
| } | |
| None => { | |
| view! { | |
| <Button | |
| text="Generate Key" | |
| on_click=move |_| generate_master_key() | |
| icon_class="ri-key-2-line" | |
| /> | |
| } | |
| } | |
| }} | |
| </PortalModal> | |
| } | |
| } | |
| None => ().into_view(), | |
| }} | |
| {move || match key_generation_rws.get() { | |
| Some(popup_state) => { | |
| view! { | |
| <PortalModal | |
| heading="Generate Master Encryption Key" | |
| handle_close=move |_| key_generation_rws.set(None) | |
| > | |
| <div class="alert alert-warning"> | |
| <i class="ri-alert-line" /> | |
| <span> | |
| "This will generate a new master encryption key. Save it securely - it will not be shown again." | |
| </span> | |
| </div> | |
| {match popup_state.clone() { | |
| Some(generated_key) => { | |
| let key_to_copy = generated_key.clone(); | |
| let copy_to_clipboard = move |_| { | |
| let key = key_to_copy.clone(); | |
| if let Some(window) = web_sys::window() { | |
| let promise = window.navigator().clipboard().write_text(&key); | |
| wasm_bindgen_futures::spawn_local(async move { | |
| match wasm_bindgen_futures::JsFuture::from(promise).await { | |
| Ok(_) => enqueue_alert( | |
| "Key copied to clipboard".to_string(), | |
| AlertType::Success, | |
| 3000, | |
| ), | |
| Err(_) => enqueue_alert( | |
| "Failed to copy key to clipboard".to_string(), | |
| AlertType::Error, | |
| 3000, | |
| ), | |
| } | |
| }); | |
| } | |
| }; | |
| view! { | |
| <div class="flex flex-col gap-2"> | |
| <div class="control-form"> | |
| <Label title="Generated Key" /> | |
| <div class="p-4 flex justify-between items-center bg-gray-900 text-green-400 rounded-lg"> | |
| <div class="font-mono text-[14px] break-all select-all"> | |
| {generated_key} | |
| </div> | |
| <button | |
| type="button" | |
| class="px-1 bg-gray-800 hover:bg-gray-700 text-white rounded transition-colors" | |
| on:click=copy_to_clipboard | |
| title="Copy to clipboard" | |
| > | |
| <i class="ri-file-copy-line" /> | |
| </button> | |
| </div> | |
| </div> | |
| <div class="control-form"> | |
| <Label title="Instructions" /> | |
| <ol class="p-4 flex flex-col gap-3 bg-base-200 rounded-lg list-none"> | |
| {key_generation_instructions() | |
| .into_iter() | |
| .enumerate() | |
| .map(|(index, instruction)| { | |
| view! { | |
| <li class="flex gap-3 items-center"> | |
| <span class="flex-shrink-0 w-6 h-6 flex items-center justify-center bg-primary text-primary-content rounded-full text-xs font-semibold"> | |
| {(index + 1).to_string()} | |
| </span> | |
| <div class="text-[14px] flex-1">{instruction}</div> | |
| </li> | |
| } | |
| }) | |
| .collect_view()} | |
| </ol> | |
| </div> | |
| </div> | |
| } | |
| .into_view() | |
| } | |
| None => ().into_view(), | |
| }} | |
| {match popup_state.clone() { | |
| Some(_) => { | |
| view! { | |
| <Button | |
| text="Close" | |
| on_click=move |_| key_generation_rws.set(None) | |
| icon_class="ri-close-line" | |
| /> | |
| } | |
| } | |
| None => { | |
| view! { | |
| <Button | |
| text="Generate Key" | |
| on_click=move |_| generate_master_key() | |
| icon_class="ri-key-2-line" | |
| /> | |
| } | |
| } | |
| }} | |
| </PortalModal> | |
| } | |
| } | |
| None => ().into_view(), | |
| }} |
🤖 Prompt for AI Agents
In `@crates/frontend/src/pages/organisations.rs` around lines 180 - 276, The
clipboard write in the copy_to_clipboard closure currently ignores that
write_text returns a JS Promise; replace the sync call with proper promise
handling by calling window.navigator().clipboard().write_text(&key), wrapping
that Promise with wasm_bindgen_futures::JsFuture::from(...) and driving it with
wasm_bindgen_futures::spawn_local so you can await it and handle errors; on
success call enqueue_alert("Key copied to clipboard", AlertType::Success, 3000)
and on failure enqueue an error alert (and optionally log the JS error). Update
the copy_to_clipboard closure in the popup_state handling (the closure that
references key_to_copy) to use spawn_local+JsFuture and import
wasm_bindgen_futures, leaving generate_master_key and key_generation_rws usage
unchanged.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change Log
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.