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

jk- create listPage #40

Merged
merged 45 commits into from
Mar 12, 2022
Merged

jk- create listPage #40

merged 45 commits into from
Mar 12, 2022

Conversation

Jacqueskim
Copy link
Contributor

@Jacqueskim Jacqueskim commented Mar 8, 2022

Overview

  • Implement and test CommonsList. Add its storybook entry. Modify the router and navigation bar to support the new page.
  • Implement and test CommonsTable. Add its storybook entry.
  • Add full coverage to commonsUtil.js.
  • Fix the form that creates commons to supply the starting date to the backend. Previously, the date was being passed to the wrong key in the POST method by the form.

Details

#12, #35, #36, #48

@codecov
Copy link

codecov bot commented Mar 8, 2022

Codecov Report

Merging #40 (1c7a062) into main (4b2e41b) will increase coverage by 0.98%.
The diff coverage is 95.83%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main      #40      +/-   ##
============================================
+ Coverage     77.80%   78.78%   +0.98%     
  Complexity       59       59              
============================================
  Files            48       51       +3     
  Lines           374      396      +22     
  Branches         10       10              
============================================
+ Hits            291      312      +21     
- Misses           83       84       +1     
Impacted Files Coverage Δ
frontend/src/main/components/Nav/AppNavbar.js 100.00% <ø> (ø)
...ontend/src/main/components/Commons/CommonsTable.js 90.90% <90.90%> (ø)
...d/src/main/components/Commons/CreateCommonsForm.js 100.00% <100.00%> (ø)
frontend/src/main/components/OurTable.js 100.00% <100.00%> (ø)
frontend/src/main/pages/AdminCreateCommonsPage.js 100.00% <100.00%> (ø)
frontend/src/main/pages/AdminListCommonPage.js 100.00% <100.00%> (ø)
frontend/src/main/utils/commonsUtils.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b2e41b...1c7a062. Read the comment docs.

pconrad
pconrad previously requested changes Mar 8, 2022
Copy link
Contributor

@pconrad pconrad left a comment

Choose a reason for hiding this comment

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

It seems there are some code coverage problems ... they are pointed out by the codecov comments on the PR.

See if you can address these.

DannyCoco
DannyCoco previously approved these changes Mar 9, 2022
@jacksoncooper jacksoncooper removed the request for review from pconrad March 10, 2022 00:19
@Jacqueskim Jacqueskim added the Attn-peer-CR At least one team member should do a code review before the staff label Mar 10, 2022
@pconrad pconrad added Attn-respond-to-CR Please address changes requested in code review Attn-Merge Conflicts and removed Merge-Candidate Staff thinks this is close to being mergeable; just waiting on either CI/CD or testing on Heroku labels Mar 12, 2022
@jacksoncooper jacksoncooper temporarily deployed to team04-w22-6pm-4-qa March 12, 2022 02:10 Inactive
@jacksoncooper jacksoncooper temporarily deployed to team04-w22-6pm-4-qa March 12, 2022 02:27 Inactive
@jacksoncooper jacksoncooper self-requested a review March 12, 2022 02:28
jacksoncooper
jacksoncooper previously approved these changes Mar 12, 2022
Copy link
Contributor

@jacksoncooper jacksoncooper left a comment

Choose a reason for hiding this comment

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

Reverted date conversion and button columns.

sahilnaik1
sahilnaik1 previously approved these changes Mar 12, 2022
Copy link

@sahilnaik1 sahilnaik1 left a comment

Choose a reason for hiding this comment

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

LGTM

@sahilnaik1 sahilnaik1 removed the Attn-respond-to-CR Please address changes requested in code review label Mar 12, 2022
@jacksoncooper jacksoncooper temporarily deployed to team04-w22-6pm-4-qa March 12, 2022 02:40 Inactive
@jacksoncooper jacksoncooper self-requested a review March 12, 2022 04:06
Copy link
Contributor

@jacksoncooper jacksoncooper left a comment

Choose a reason for hiding this comment

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

Now with 📕.

@pconrad pconrad temporarily deployed to team04-w22-6pm-4-qa March 12, 2022 05:18 Inactive
@sahilnaik1 sahilnaik1 temporarily deployed to team04-w22-6pm-4-qa March 12, 2022 05:20 Inactive
@pconrad pconrad added Merge-Candidate Staff thinks this is close to being mergeable; just waiting on either CI/CD or testing on Heroku 10 and removed Merge-Candidate Staff thinks this is close to being mergeable; just waiting on either CI/CD or testing on Heroku labels Mar 12, 2022
@pconrad pconrad merged commit fbf08f8 into main Mar 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6pm-4: Create List page, Make commonsTable, Make commonsUtil, and tests for each
7 participants