Feature/userinfo - #273
Feature/userinfo#273JasonRobertFrancis wants to merge 73 commits into
Conversation
Bundle ReportChanges will increase total bundle size by 1.07kB (0.05%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: viper-frontend-esmAssets Changed:
Files in
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #273 +/- ##
==========================================
- Coverage 45.36% 43.74% -1.62%
==========================================
Files 948 1108 +160
Lines 49529 53560 +4031
Branches 6696 7043 +347
==========================================
+ Hits 22467 23431 +964
- Misses 26096 29086 +2990
- Partials 966 1043 +77
Flags with carried forward coverage won't be shown. Click here to find out more.
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
This comment was marked as resolved.
This comment was marked as resolved.
…modal) and responds to code-quality bot userinfo recommendations
22567dc to
694ecd4
Compare
This comment was marked as resolved.
This comment was marked as resolved.
For the effective date, it looks like this is pulling the effective date of the last change to the position. It might make more sense to show the effdt on the job record (the effective date of the last change to the job) in the summary and history views. Employees have an original hire date, but not a hire date to a specific job. I would also suggest removing the reports to column from the uc path history. I removed this from the uc path view on the current directory because it can be ambiguous when looking at historical data (which is why it looks like Rex is reporting to Dan). I checked Rex's permissions on test and the counts look good. The prod user info page is double counting some permissions. |
|
@JasonRobertFrancis, I've resolved all my open comment threads. Some minor issues:
|
rlorenzo
left a comment
rlorenzo
left a comment
There was a problem hiding this comment.
@JasonRobertFrancis c3de7d7 "Represses Instinct error if API is not available" is on Development only, not on feature/userinfo. It is the commit that actually suppresses the Instinct error on TEST (IsDevelopmentEnvironment to IsInstinctOptionalEnvironment, covering Test as well as Development). Since Development is never a base and never merges to main, that change disappears when this PR merges. Please cherry-pick it onto feature/userinfo and re-merge into Development.
| "commandName": "Project", | ||
| "dotnetRunMessages": true, | ||
| "launchBrowser": false, | ||
| "launchBrowser": true, |
There was a problem hiding this comment.
Still flipped false to true versus main, on the https profile. npm run dev opens the browser already, so this gives you two tabs. Unrelated to the feature.
There was a problem hiding this comment.
@JasonRobertFrancis, I've resolved all my other code review comments except this one.
Brandon and the Copilot/GitHub review bots still have some open code review comments.
Resolves ReSharper S8969 warnings flagged by the PR-scoped gate: the compiler already narrows InstinctInfo.ErrorMessage to non-null after the preceding Assert.NotNull check, so the ! was redundant.
No description provided.