Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User Privacy Settings #2031

Open
wants to merge 27 commits into
base: develop
Choose a base branch
from
Open

User Privacy Settings #2031

wants to merge 27 commits into from

Conversation

ArabellaJi
Copy link
Contributor

@ArabellaJi ArabellaJi commented Jul 14, 2023

Note: This is the new PR for #1931. The reason for creating this new PR is that when I wanted to merge develop into my old branch, I mistakenly did a git rebase develop in that branch, which pushed some irrelevant changes and commits made by other people into that old branch. Even though those changes were not harmful, they could still lead to some confusion, so I decided to open this new branch and PR.

Allow users to choose which phone numbers they would like to show in their profiles and some other privacy options.

Fixes #1308
Fixes #1897

API: gordon-cs/gordon-360-api#1042

@ArabellaJi ArabellaJi changed the title New PR for "S23 fac privacy option" New PR for "S23 user privacy setting" Jul 17, 2023
@amos-cha
Copy link
Contributor

handle merge conflicts then I'll review

amos-cha
amos-cha previously approved these changes Jul 21, 2023
@ArabellaJi
Copy link
Contributor Author

ArabellaJi commented Jul 21, 2023

NOTE: Don't merge until Database and API is pushed and merged!

@bennettforkner bennettforkner changed the title New PR for "S23 user privacy setting" User Privacy Settings Oct 10, 2023
@@ -43,7 +44,7 @@ const formatPhone = (phone) => {

const PersonalInfoList = ({ myProf, profile, isOnline, createSnackbar }) => {
const [isMobilePhonePrivate, setIsMobilePhonePrivate] = useState(
Boolean(profile.IsMobilePhonePrivate && profile.MobilePhone !== PRIVATE_INFO),
Boolean(profile.IsMobilePhonePrivate || profile.MobilePhone === PRIVATE_INFO),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this ever not a boolean where we need a cast?

privateInfo={isMobilePhonePrivate}
myProf={myProf}
/>
) : null;

let streetAddr = profile.HomeStreet2 ? <span>{profile.HomeStreet2},&nbsp;</span> : null;

let combineHomeLocation =
profile.Country === 'United States of America' || !profile.Country
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
profile.Country === 'United States of America' || !profile.Country
profile.Country === 'United States of America' || !profile.Country

Second time this line comes up. Potentially better to extract it into its own variable that is used both here and in the home variable below

Comment on lines +1 to +18
// vite.config.ts
import react from "file:///nfs/users/arabella.ji/summer-practicum/gordon-360-ui/node_modules/@vitejs/plugin-react/dist/index.mjs";
import { defineConfig, loadEnv, splitVendorChunkPlugin } from "file:///nfs/users/arabella.ji/summer-practicum/gordon-360-ui/node_modules/vite/dist/node/index.js";
import viteTsconfigPaths from "file:///nfs/users/arabella.ji/summer-practicum/gordon-360-ui/node_modules/vite-tsconfig-paths/dist/index.mjs";
var config = ({ mode }) => {
process.env = { ...process.env, ...loadEnv(mode, process.cwd()) };
return defineConfig({
build: {
target: "es2022"
},
plugins: [react(), viteTsconfigPaths(), splitVendorChunkPlugin()],
server: {
port: parseInt(process.env.VITE_PORT ?? "5173"),
host: process.env.VITE_HOST ?? "localhost",
proxy: {
"/api": {
target: process.env.VITE_API_URL,
changeOrigin: true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this file and is it necessary to be added?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this file included in your PR? It seems unrelated to me.

@bennettforkner
Copy link
Contributor

Yea there are a lot of files from other peoples' branches in your PR. The issues will be connected to the branch, not the PR.

My recommendation would be that you create a new branch based on develop and use git cherry-pick to pull the specific commits that you want to be a part of the PR.
i.e.

git switch develop
git checkout -b s23-fac-privacy-option-ui-3
git cherry-pick [sha hashes of commits you want to merge]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add more granular Privacy Options for Fac/Staff Profile Show mobile phone # for facstaff
4 participants