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

refactor: add pub #55

Merged
merged 40 commits into from
Feb 15, 2024
Merged

refactor: add pub #55

merged 40 commits into from
Feb 15, 2024

Conversation

eldu
Copy link
Contributor

@eldu eldu commented Jan 24, 2024

  • deprecates keycloak, redux-saga
  • refactors add pub form into modal
    • changes search to only use doi, and not also doi url. Decision was made to streamline rather than support multiple inputs
    • valid searched publication populates the manual form for users to verify and edit
    • adds yup validation and formik error messages on all fields
    • handles whether publication already exists in our database
    • handles results from fetching publication
    • handles undefined values by basically setting every field to have to be defined (firebase hates undefined)
  • refactors firebase publication document into a standalone document per publication
  • refactors fetching publications into a custom react hook that watches the publications collection and automatically updates the redux store
  • loading and disabled form
  • fix css weirdness... i'm so sorry this is in this pr

⚠️ Temporary security risk: I updated the firebase rules for the "publications" collection to allow anyone to write to it ⚠️
Until #49

Search Form
Manual Form

@eldu eldu changed the base branch from main to feat-lint January 24, 2024 21:17
Copy link

github-actions bot commented Jan 24, 2024

Visit the preview URL for this PR (updated for commit c702ab0):

https://ccv-pubs--pr55-feat-add-pub-qqw9yxv6.web.app

(expires Thu, 15 Feb 2024 15:56:12 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f75c16b3bdd7d81b7a0b7f8d3eed826b541e7a2a

@eldu eldu changed the base branch from feat-lint to build-update January 25, 2024 23:44
@eldu eldu linked an issue Feb 6, 2024 that may be closed by this pull request
@eldu eldu self-assigned this Feb 6, 2024
@eldu eldu marked this pull request as ready for review February 6, 2024 14:14
@eldu eldu marked this pull request as draft February 6, 2024 21:56
@eldu
Copy link
Contributor Author

eldu commented Feb 6, 2024

Returning to draft because I want to fix the reads

@eldu eldu marked this pull request as ready for review February 7, 2024 19:23
@eldu eldu marked this pull request as draft February 7, 2024 19:26
@eldu
Copy link
Contributor Author

eldu commented Feb 7, 2024

There are 633 documents. On refresh, it is expected that they all get read. So I'm unblocking this PR.

Perhaps making a new issue on querying subsets of publications rather than all of the publications?

@eldu eldu marked this pull request as ready for review February 7, 2024 20:12
@eldu eldu requested a review from anna-murphy February 7, 2024 20:12
@eldu
Copy link
Contributor Author

eldu commented Feb 7, 2024

I added in some date stuff such that it shows the 10 most recent publications added to the database. Mostly so that this can show off the things I did in this PR for adding in the publication. So when you add you'll see the page automatically update!

Copy link

@hetd54 hetd54 left a comment

Choose a reason for hiding this comment

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

Looks amazing! My only thought is that the DOI probably does not need to be sortable.

This is probably something you will implement in the future, but noting that on mobile it's a bit cramped:
Screenshot 2024-02-14 at 3 52 07 PM

@eldu
Copy link
Contributor Author

eldu commented Feb 14, 2024

Looks amazing! My only thought is that the DOI probably does not need to be sortable.

This is probably something you will implement in the future, but noting that on mobile it's a bit cramped: Screenshot 2024-02-14 at 3 52 07 PM

I'll add this to this issue when the table gets reworked into tanstack ! #51

@eldu eldu merged commit c6e490c into build-update Feb 15, 2024
2 checks passed
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.

Refactor AddPub
2 participants