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

12 filters in url param #210

Open
wants to merge 5 commits into
base: development
Choose a base branch
from
Open

Conversation

krish1925
Copy link
Collaborator

@krish1925 krish1925 commented Oct 22, 2024

Solves issue number #12 . Tested on different browsers.

Copy link

gitguardian bot commented Oct 22, 2024

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id GitGuardian status Secret Commit Filename
14243322 Triggered Generic Password d6f6db3 app/components/SignIn.js View secret
14243322 Triggered Generic Password 0562119 app/components/SignIn.js View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@@ -2,8 +2,8 @@
"editor.formatOnSave": true,
"editor.defaultFormatter": "esbenp.prettier-vscode",
"editor.codeActionsOnSave": {
"source.fixAll": true, // Auto-fix ESLint issues on save
"source.fixAll.eslint": true // Specific to ESLint
"source.fixAll": "explicit",
Copy link
Collaborator

Choose a reason for hiding this comment

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

make sure not to commit your vscode settings as people will have different configs. the current one auto-formats for prettier/eslint on save so revert this change please

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I had this installed for some other project and accidentally committed this to repo, I'll make sure not to commit that

@@ -5,6 +5,7 @@ import styles from '../styles/FilterList.module.css'
// This file renammes columns to more human-readable names
import nameMap from '../services/nameMap.js'

// need to add functionality such that the
Copy link
Collaborator

Choose a reason for hiding this comment

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

such that the ...?

if (initialFilters.length > 0) {
setFilterList(initialFilters)
}
}, [setFilterList])
Copy link
Collaborator

Choose a reason for hiding this comment

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

change the dependency list to []. This is more clear and runs on the initial render.

This is also buggy: If I go to http://localhost:3000/matches/Z6F1Amn51528fzmVlHlV?player1ServePlacement=Deuce%3A+Body the filters are not applied. I have to click on the filters tab, then the state is updated.

@Fredenck
Copy link
Collaborator

make sure you run git merge development to resolve merge conflicts

@ritzz26
Copy link
Collaborator

ritzz26 commented Nov 19, 2024

@Fredenck @krish1925 is this still something we are working on ?

@Fredenck
Copy link
Collaborator

talked to Krish about it, but here's the tldr: yes still being worked on, some more testing needs to be done after the recent changes (deletion of extractSetScores), new Firebase data format, etc

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.

3 participants