[RHOAIENG-6867] Tracking of dashboard version and users's capabilities #2878
Conversation
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
pilhuhn
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2878 +/- ##
==========================================
+ Coverage 78.62% 78.72% +0.10%
==========================================
Files 1120 1105 -15
Lines 23757 23519 -238
Branches 5979 5942 -37
==========================================
- Hits 18679 18516 -163
+ Misses 5078 5003 -75
... and 54 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
|
/retest-required |
There was a problem hiding this comment.
I'm noticing some extra stuff here as I'm trying to look at approving your work here -- see my new review questions.
Also -- We'll need to look to get you into our scrum. Your ticket is now moved to the sprint but you're going to need to attend and communicate through our scrum.
Please direct any questions to our UXD Coordinator, @jeff-phillips-18
cc @dgutride
| const updateReviewResource: AccessReviewResourceAttributes = { | ||
| group: 'datasciencecluster.opendatahub.io/v1', | ||
| resource: 'DataScienceCluster', | ||
| verb: 'update', | ||
| }; | ||
| const [allowUpdate, updateLoaded] = useAccessReview(updateReviewResource); |
There was a problem hiding this comment.
Not sure I understand this request of metadata. What are you aiming at here?
| // TODO this should go to a helper, as it is called from multiple places | ||
| const createReviewResource: AccessReviewResourceAttributes = { | ||
| group: 'project.openshift.io', | ||
| resource: 'projectrequests', | ||
| verb: 'create', | ||
| }; | ||
| const [allowCreate, createLoaded] = useAccessReview(createReviewResource); | ||
|
|
||
| // TODO this should go to a helper, as it is called from multiple places | ||
| const deleteReviewResource: AccessReviewResourceAttributes = { | ||
| group: 'project.openshift.io', | ||
| resource: 'projectrequests', | ||
| verb: 'delete', | ||
| }; | ||
| const [allowDelete, deleteLoaded] = useAccessReview(deleteReviewResource); |
There was a problem hiding this comment.
This feels like you're asking if they are self provisioners? Why do you care if they can delete? I don't think anyone deletes project requests... they delete the project behind it.
What are you trying to capture here?
| canCreate: allowCreate, | ||
| canDelete: allowDelete, | ||
| canUpdate: allowUpdate, |
There was a problem hiding this comment.
These are very vague and you're sending them on init? Shouldn't we just make a "load request" at some point in the future and record user's permissions at startup instead of mixing it into the infra startup of the segment tool?
There was a problem hiding this comment.
The analytics.identify() happens within init. And identify() is the place to send user-properties.
We can for sure send that any time later. As this is only needed once and not on every page load, init looks like a good place for me.
There was a problem hiding this comment.
Our segment tracking was haphazardly put together and is still in rough shape. The more you add to it the more I want to spend time making sure we are doing the right thing, structured and future looking.
init is init of the infrastructure and should be a cohesive set of "startup and mounting of segment" statements. It shouldn't manage the initial state of the logged in user, that should be done via another hook once things have started up.
I'd like to start seeing proper documentation and setup for these things... maybe even a src/concepts/segment folder where we build out utilities to use it throughout our app.
|
I've broken out the dashboard version part in #2896 |
|
I am closing this in favor of a better solution going forward. |
Description
This is related to RHOAIENG-6867
How Has This Been Tested?
Manually be looking at the data that arrived in Segment / by looking at request in the Browsers network tab
Test Impact
No change on functionality of the visible UI
Request review criteria:
Self checklist (all need to be checked):
After the PR is posted & before it merges:
main