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

Align frontend for PD CRUD to be similar to the schools page #44

Merged
merged 8 commits into from
Mar 22, 2024

Conversation

perryzjc
Copy link
Member

Pivotal Tracker Link

What This PR Does:

This PR accomplishes the following tasks:

  1. Creates CRUD frontend pages for PD (Professional Developments) along with frontend validations for a better UX experience, such as value presence check, prompt, auto-fill, etc.
  2. Creates mockups for some backend data (available PDs, registrations).
  3. Sets up mockup routes with a flash message indicating "feature not implemented."
  4. Currently, it doesn't have any tests since its backend database and data are highly mockup right now, making it unable to use many database features for cucumber tests (such as expect(Teacher.exists?(email:)).to be false). Tests should be created after the model, schema, and controllers are properly set up.

This pull request fixes|implements (pick one...) implements.

Include screenshots, videos, etc.

Overall Pages - Index Page

image

Overall Pages - Create New PD Page

image

Overall Pages - Edit PD Page

image

Overall Pages - Show Page

image

Frontend Behaviors - Add Teacher at Show Page

image After hit `submit`, current mockup behavior for controller is to show `Create feature is not yet implemented.` image

Frontend Behaviors - Update Teacher at Show Page

It automatically fills in the data from the selected record to the modal.
image
After hit submit, current mockup behavior for controller is to show Update feature is not yet implemented.
image

Frontend Behaviors - Delete Teacher at Show Page

After hit , current mockup behavior for controller is to show Destroy feature is not yet implemented.
image

Frontend Behaviors - Delete/Update PD itself (not teacher)

Same as before, they all gonna show as XX feature is not yet implemented.
image
image

Who authored this PR?

  1. Perry (Jingchao) Zhong
  2. Michael Tao (Pair Programed with Perry)

How should this PR be tested?

Test its frontend behaviors & ability to integrate with backends

  • Is there a deploy we can view?
  • What do the specs/features test?
  • Are there edge cases to watch out for?

Are there any complications to deploying this?

No. It's just a new frontend feature and does not break any existing test.

Checklist:

  • Has this been deployed to a staging environment or reviewed by a customer?
  • Tag someone for code review (either a coach / team member)
  • I have renamed the branch to match PivotTracker's suggested one (necessary for BlueJay) (e.g. michael/12345-add-new-feature Any branch name will do as long as the story ID is there. You can use git checkout -b [new-branch-name])

Copy link

codecov bot commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 0% with 142 lines in your changes are missing coverage. Please review.

Project coverage is 71.56%. Comparing base (71b92b6) to head (669e1a2).

Files Patch % Lines
...ontrollers/professional_developments_controller.rb 0.00% 79 Missing ⚠️
app/models/professional_development.rb 0.00% 36 Missing ⚠️
app/controllers/pd_registrations_controller.rb 0.00% 14 Missing ⚠️
app/models/pd_registration.rb 0.00% 13 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##           pd-frontend+backend      #44       +/-   ##
========================================================
- Coverage                86.18%   71.56%   -14.63%     
========================================================
  Files                       20       24        +4     
  Lines                      695      837      +142     
========================================================
  Hits                       599      599               
- Misses                      96      238      +142     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ArushC
Copy link

ArushC commented Mar 21, 2024

@perryzjc @realmichaeltao everything looks good overall. A couple of comments:

  1. I found the Javascript at the bottom of app/views/professional_developments/show.html.erb to be extremely long and confusing and so I think that it could benefit from some helpful code comments. And it might be possible to refactor some bits using the more concise jQuery $(...) notation.

  2. Due to the lack of testing, I do NOT believe this is ready to be merged, but I understand that a lot of stuff on this branch cannot be tested till the backend features get implemented. Therefore I would suggest that @razztech @JacksonXu33 get started on the backend directly working on this branch and put together the features/tests.

@JacksonXu33
Copy link

I'm just going to merge this branch to frontend+backend branch so that backend can branch off of this.

@JacksonXu33 JacksonXu33 merged commit 8117d90 into pd-frontend+backend Mar 22, 2024
10 checks passed
@JacksonXu33 JacksonXu33 deleted the 187242903/features/pd-frontend branch March 22, 2024 20:28
@JacksonXu33 JacksonXu33 restored the 187242903/features/pd-frontend branch March 22, 2024 20:28
@cycomachead cycomachead deleted the 187242903/features/pd-frontend branch May 8, 2024 08:58
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