feat: implement expert availability toggle, transparency log, and rent estimation#385
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThree independent features are added: an ChangesExpert Availability Toggle Widget
Transparency Audit Log Page
State Rent Fee Estimation in FundSessionModal
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: one or more packages not found in the registry. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/transparency/page.tsx`:
- Around line 52-72: The ADMIN_EVENTS rendering in the transparency page is not
ordered consistently, so audit entries can appear out of sequence. Before
mapping in the table render, sort the ADMIN_EVENTS data explicitly by timestamp
in a single deterministic order (newest-first or oldest-first) using the
existing page component logic around the ADMIN_EVENTS.map block. Ensure the
sorted collection is what gets rendered, not the original unsorted array.
- Line 55: The timestamp rendering in the transparency page currently uses
Date.prototype.toLocaleString, which varies by runtime locale and omits timezone
context. Update the event timestamp display in the transparency page component
to use a fixed, explicit timezone format such as UTC so the same event renders
consistently across environments, and keep the change localized to the JSX that
formats event.timestamp.
- Around line 3-28: The transparency page is still using the hardcoded
ADMIN_EVENTS sample array, so it will display stale or fictitious admin actions
instead of a real public record. Replace ADMIN_EVENTS in the page component with
data fetched from the actual event source (or a server-side data loader) and
render that source dynamically in the existing admin-events UI. Make sure the
page uses real caller addresses, tx hashes, timestamps, and action details
rather than static placeholders.
In `@src/components/dashboard/ExpertStats.tsx`:
- Around line 24-35: The availability toggle in ExpertStats should expose its
current state to assistive tech, since it is currently only labeled and not
announced as on or off. Update the button in the toggle UI to use proper switch
semantics in the component that renders handleToggle/isAvailable, such as adding
role="switch" with aria-checked bound to isAvailable (or equivalent pressed
state), while keeping the existing aria-label and visual behavior unchanged.
- Around line 5-10: The availability toggle in ExpertStats is currently only
kept in local component state, so it resets on remount and does not meet the
persistent widget requirement. Update the ExpertStats component to hydrate and
save the flag from stable client storage using the same local-storage approach
already used elsewhere in the dashboard, and make handleToggle update both the
stored value and the React state so the status survives refreshes.
In `@src/components/marketplace/FundSessionModal.tsx`:
- Around line 45-47: The rent estimation fallback in FundSessionModal should not
use a fake 0.01 XLM value when the dry-run fails, because that lets
totalRequired and the balance check proceed with an incorrect fee. Update the
rent-estimation flow in the relevant estimate/rent-fee logic so failures leave
the estimate unavailable, surface an error state, and prevent confirmation until
a real RPC-backed fee is obtained. Use the existing setRentFee and the
surrounding checkout/confirmation logic in FundSessionModal to block the action
instead of continuing with a placeholder.
- Around line 31-57: The rent estimation flow in FundSessionModal can be
overwritten by stale async completions, causing isEstimatingRent and rentFee to
reflect an older request and briefly enabling checkout with a 0 XLM total.
Update fetchRentEstimate and the React.useEffect trigger so each duration/isOpen
change creates a fresh request identity (or cancels prior requests) and only the
latest request is allowed to clear loading or set rentFee. Also set the
estimating state synchronously before kicking off the request so the initial
render and rapid duration changes never show a false-ready Continue state.
- Around line 173-179: The XLM amount rows in FundSessionModal are mixing fiat
and token labels by prefixing totals with "$" while still rendering "XLM";
update the rendering in the estimated rent and total amount sections so they use
a single consistent currency label. Locate the amount display logic in
FundSessionModal and either remove the dollar sign for XLM-denominated values or
convert the value to fiat before keeping "$", and apply the same fix to the
other affected amount block mentioned in the comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 43de8ce6-f223-45c0-83c3-7429b2108747
📒 Files selected for processing (4)
src/app/transparency/page.tsxsrc/components/dashboard/ExpertStats.tsxsrc/components/dashboard/Header.tsxsrc/components/marketplace/FundSessionModal.tsx
| const ADMIN_EVENTS = [ | ||
| { | ||
| id: "1", | ||
| timestamp: "2024-06-25T14:30:00Z", | ||
| action: "Set Platform Fee", | ||
| details: "Fee updated to 2.5%", | ||
| caller: "GBXYZ...ABCD", | ||
| txHash: "9a8b7c6d5e4f3a2b1c0d...", | ||
| }, | ||
| { | ||
| id: "2", | ||
| timestamp: "2024-06-20T09:15:00Z", | ||
| action: "Pause Contract", | ||
| details: "Emergency pause activated", | ||
| caller: "GBXYZ...ABCD", | ||
| txHash: "1a2b3c4d5e6f7a8b9c0d...", | ||
| }, | ||
| { | ||
| id: "3", | ||
| timestamp: "2024-06-21T10:00:00Z", | ||
| action: "Unpause Contract", | ||
| details: "Operations resumed", | ||
| caller: "GBXYZ...ABCD", | ||
| txHash: "fedcba0987654321...", | ||
| }, | ||
| ] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Replace the hardcoded audit log with a real event source.
This page claims to be the public record of contract admin actions, but ADMIN_EVENTS is just static sample data. Shipping it as-is will show fictitious/stale caller addresses and tx hashes, which defeats the transparency feature.
Also applies to: 35-36
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/transparency/page.tsx` around lines 3 - 28, The transparency page is
still using the hardcoded ADMIN_EVENTS sample array, so it will display stale or
fictitious admin actions instead of a real public record. Replace ADMIN_EVENTS
in the page component with data fetched from the actual event source (or a
server-side data loader) and render that source dynamically in the existing
admin-events UI. Make sure the page uses real caller addresses, tx hashes,
timestamps, and action details rather than static placeholders.
| {ADMIN_EVENTS.map((event) => ( | ||
| <tr key={event.id} className="hover:bg-slate-800/30 transition-colors"> | ||
| <td className="px-6 py-4 text-slate-400 whitespace-nowrap"> | ||
| {new Date(event.timestamp).toLocaleString()} | ||
| </td> | ||
| <td className="px-6 py-4"> | ||
| <div className="font-medium text-white">{event.action}</div> | ||
| <div className="text-xs text-slate-500 mt-1">{event.details}</div> | ||
| </td> | ||
| <td className="px-6 py-4"> | ||
| <span className="font-mono text-purple-400 bg-purple-400/10 px-2 py-1 rounded"> | ||
| {event.caller} | ||
| </span> | ||
| </td> | ||
| <td className="px-6 py-4"> | ||
| <span className="font-mono text-slate-300"> | ||
| {event.txHash} | ||
| </span> | ||
| </td> | ||
| </tr> | ||
| ))} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Render the audit entries in a deterministic time order.
The current list is already out of sequence (2024-06-20 is rendered after 2024-06-25, then 2024-06-21). Audit logs need an explicit newest-first or oldest-first sort before mapping.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/transparency/page.tsx` around lines 52 - 72, The ADMIN_EVENTS
rendering in the transparency page is not ordered consistently, so audit entries
can appear out of sequence. Before mapping in the table render, sort the
ADMIN_EVENTS data explicitly by timestamp in a single deterministic order
(newest-first or oldest-first) using the existing page component logic around
the ADMIN_EVENTS.map block. Ensure the sorted collection is what gets rendered,
not the original unsorted array.
| {ADMIN_EVENTS.map((event) => ( | ||
| <tr key={event.id} className="hover:bg-slate-800/30 transition-colors"> | ||
| <td className="px-6 py-4 text-slate-400 whitespace-nowrap"> | ||
| {new Date(event.timestamp).toLocaleString()} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Format timestamps with an explicit timezone.
toLocaleString() here depends on the server/runtime locale and omits the timezone, so the same on-chain event can be shown differently across environments. For an audit surface, render a fixed format (for example UTC) instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/transparency/page.tsx` at line 55, The timestamp rendering in the
transparency page currently uses Date.prototype.toLocaleString, which varies by
runtime locale and omits timezone context. Update the event timestamp display in
the transparency page component to use a fixed, explicit timezone format such as
UTC so the same event renders consistently across environments, and keep the
change localized to the JSX that formats event.timestamp.
| const [isAvailable, setIsAvailable] = useState(false) | ||
|
|
||
| const handleToggle = () => { | ||
| // Simulated on-chain heartbeat update | ||
| setIsAvailable(!isAvailable) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Persist the availability state instead of keeping it component-local.
Line 5 always reinitializes the toggle to false, so the status resets after a refresh/remount and misses the “persistent dashboard widget” requirement. Store and hydrate this flag from stable client storage (for example the same local-storage pattern already used in the dashboard header).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/dashboard/ExpertStats.tsx` around lines 5 - 10, The
availability toggle in ExpertStats is currently only kept in local component
state, so it resets on remount and does not meet the persistent widget
requirement. Update the ExpertStats component to hydrate and save the flag from
stable client storage using the same local-storage approach already used
elsewhere in the dashboard, and make handleToggle update both the stored value
and the React state so the status survives refreshes.
| <button | ||
| onClick={handleToggle} | ||
| className={`relative inline-flex h-6 w-11 items-center rounded-full transition-colors ${ | ||
| isAvailable ? "bg-green-500/30" : "bg-slate-700" | ||
| }`} | ||
| aria-label="Toggle availability" | ||
| > | ||
| <span | ||
| className={`inline-block h-4 w-4 transform rounded-full bg-white transition-transform ${ | ||
| isAvailable ? "translate-x-6" : "translate-x-1" | ||
| }`} | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Expose the current toggle state to assistive tech.
This is implemented as a switch, but the button only announces “Toggle availability” and not whether it is currently on or off. Add switch/toggle state semantics such as role="switch" with aria-checked={isAvailable} (or aria-pressed).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/dashboard/ExpertStats.tsx` around lines 24 - 35, The
availability toggle in ExpertStats should expose its current state to assistive
tech, since it is currently only labeled and not announced as on or off. Update
the button in the toggle UI to use proper switch semantics in the component that
renders handleToggle/isAvailable, such as adding role="switch" with aria-checked
bound to isAvailable (or equivalent pressed state), while keeping the existing
aria-label and visual behavior unchanged.
| const [rentFee, setRentFee] = useState<number | null>(null); | ||
| const [isEstimatingRent, setIsEstimatingRent] = useState(false); | ||
|
|
||
| const hourlyRate = parseInt(expertHourlyRate?.replace(/\D/g, '') || '50'); | ||
| const amount = (hourlyRate * duration) / 60; | ||
| const sessionId = `SESSION_${Date.now()}`; | ||
|
|
||
| // Simulate RPC call for state rent | ||
| const fetchRentEstimate = async () => { | ||
| setIsEstimatingRent(true); | ||
| try { | ||
| // Dummy RPC dry-run simulation | ||
| await new Promise((resolve) => setTimeout(resolve, 800)); | ||
| setRentFee(0.015); // Mock rent fee in XLM | ||
| } catch (e) { | ||
| console.error("Failed to estimate rent fee", e); | ||
| setRentFee(0.01); // Fallback mock fee | ||
| } finally { | ||
| setIsEstimatingRent(false); | ||
| } | ||
| }; | ||
|
|
||
| React.useEffect(() => { | ||
| if (isOpen) { | ||
| fetchRentEstimate(); | ||
| } | ||
| }, [isOpen, duration]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Track the latest rent-estimate request before re-enabling checkout.
Lines 38-57 can overlap when the user changes duration quickly. An older fetchRentEstimate() completion always runs setIsEstimatingRent(false), so cannotProceed can flip back to false while a newer estimate is still pending. The first open also renders once with rentFee === null and isEstimatingRent === false before the effect fires, briefly showing a 0 XLM total and an enabled Continue button. Tie the loading/result state to a request id (or cancel stale requests) and mark estimation pending synchronously when isOpen/duration changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/marketplace/FundSessionModal.tsx` around lines 31 - 57, The
rent estimation flow in FundSessionModal can be overwritten by stale async
completions, causing isEstimatingRent and rentFee to reflect an older request
and briefly enabling checkout with a 0 XLM total. Update fetchRentEstimate and
the React.useEffect trigger so each duration/isOpen change creates a fresh
request identity (or cancels prior requests) and only the latest request is
allowed to clear loading or set rentFee. Also set the estimating state
synchronously before kicking off the request so the initial render and rapid
duration changes never show a false-ready Continue state.
| } catch (e) { | ||
| console.error("Failed to estimate rent fee", e); | ||
| setRentFee(0.01); // Fallback mock fee |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Treat rent-estimation failures as blocking, not as 0.01 XLM.
If the dry-run fails here, totalRequired and the insufficient-balance check still use this made-up fee as if it came from RPC. That can understate the funds required and let an underfunded checkout continue with the wrong total. Keep the estimate unavailable, surface an error, and block confirmation until a real estimate succeeds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/marketplace/FundSessionModal.tsx` around lines 45 - 47, The
rent estimation fallback in FundSessionModal should not use a fake 0.01 XLM
value when the dry-run fails, because that lets totalRequired and the balance
check proceed with an incorrect fee. Update the rent-estimation flow in the
relevant estimate/rent-fee logic so failures leave the estimate unavailable,
surface an error state, and prevent confirmation until a real RPC-backed fee is
obtained. Use the existing setRentFee and the surrounding checkout/confirmation
logic in FundSessionModal to block the action instead of continuing with a
placeholder.
| <div className="flex justify-between text-sm"> | ||
| <span className="text-gray-400">Estimated State Rent Fee</span> | ||
| <span>{isEstimatingRent ? '...' : rentFee ? `${rentFee.toFixed(3)} XLM` : '0 XLM'}</span> | ||
| </div> | ||
| <div className="border-t border-purple-500/20 pt-3 flex justify-between font-bold"> | ||
| <span>Total Amount</span> | ||
| <span className="text-purple-400">${amount.toFixed(2)} XLM</span> | ||
| <span className="text-purple-400">${(amount + (rentFee || 0)).toFixed(3)} XLM</span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a single currency label for the XLM totals.
These rows currently render values like $50.015 XLM, which mixes fiat and XLM in the same amount. Show either 50.015 XLM or convert to a real fiat value before using $.
Also applies to: 209-215
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/marketplace/FundSessionModal.tsx` around lines 173 - 179, The
XLM amount rows in FundSessionModal are mixing fiat and token labels by
prefixing totals with "$" while still rendering "XLM"; update the rendering in
the estimated rent and total amount sections so they use a single consistent
currency label. Locate the amount display logic in FundSessionModal and either
remove the dollar sign for XLM-denominated values or convert the value to fiat
before keeping "$", and apply the same fix to the other affected amount block
mentioned in the comment.
|
Hello @Dannyorji |
|
@Luluameh |
2 participants
Description
This pull request introduces several key features for the SkillSphere-Dapp:
Changed Files
src/components/dashboard/ExpertStats.tsx(New)src/components/dashboard/Header.tsx(Updated)src/app/transparency/page.tsx(New)src/components/marketplace/FundSessionModal.tsx(Updated)Closes #330
Closes #331
Closes #332
Closes #334
Summary by CodeRabbit
New Features
Bug Fixes