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

Jump to bottom

New hire form flow #775 #818

Merged
merged 72 commits into from Mar 23, 2023
Merged

New hire form flow #775 #818

merged 72 commits into from Mar 23, 2023

Conversation

MikeRayUX
Copy link
Contributor

@MikeRayUX MikeRayUX commented Mar 18, 2023 •
edited

This PR adds Flow for businesses to notify RailsDevs when they make a hire #764

  • Adds a "Made a hire?" link to the nav bar
  • Clicking the link shows a form that looks like /hired/forms/new

@MikeRayUX MikeRayUX marked this pull request as draft March 18, 2023 17:52
@MikeRayUX
Copy link
Contributor Author

MikeRayUX commented Mar 18, 2023 •
edited

This PR contains prev commit progress by @RomanTurner from #775 (addresses #764) and serves as my starting point.

@MikeRayUX MikeRayUX marked this pull request as ready for review March 18, 2023 23:19
@MikeRayUX MikeRayUX marked this pull request as draft March 18, 2023 23:59
@MikeRayUX
Copy link
Contributor Author

MikeRayUX commented Mar 19, 2023 •
edited

The current naming structure for business_forms/developer_forms:

'hire' = business_forms (Business hired a developer)
'hired' = developer_forms (Developer found a job)

Both hire/hired follow their own namespace.

Thus far, only i18n keys/values have been updated via this suggestion

Would you also like to update the namespaces as well?

@joemasilotti
Copy link
Owner

joemasilotti commented Mar 19, 2023 •
edited

Would you also like to update the namespaces as well?

That would be great, thank you! Same goes for the routes, too.

@MikeRayUX
Copy link
Contributor Author

That would be great, thank you! Same goes for the routes, too.

Great. On it!

MikeRayUX and others added 13 commits March 22, 2023 11:48
@MikeRayUX @joemasilotti
Update config/locales/en.yml
7146a11 
Co-authored-by: Joe Masilotti <joe@masilotti.com>
@MikeRayUX
restore spacing in user_component
bbec47c
@MikeRayUX
add full_name for hiring_invoice_requests/show to i18n
e482d5c
@MikeRayUX
revert change to open_tag i18n key
f786e1c
@MikeRayUX
remove unused key from en.yml
ae60d71
@MikeRayUX
fixture: foreign key error
7c45c76
@MikeRayUX
remove admin namespace from admin/businesses/hiring_invoice_requests/…
e648e38 
…notifications.rb
@MikeRayUX
ensure hiring_invoice_request_notification i18n key matches class name
f11aa8c 
remove outdated notifications concern
@MikeRayUX
move user hiring_invoice_requests route declaration
efa3081
@MikeRayUX
reuse attributes from HiringInvoiceRequestsHelper in HiringInvoiceReq…
e8b7ba8 
…uestsTest
@MikeRayUX
add test_helper import to Businesses::HiringInvoiceRequests::Notifica…
d630a6d 
…tion::NotificationsTest
@MikeRayUX
remove admin namespace from HiringInvoiceRequestsHelper
93591ef
@MikeRayUX
restore null: false to migration
ae362ed
@MikeRayUX
Copy link
Contributor Author

@joemasilotti Ready for review!

@joemasilotti
Copy link
Owner

Can you enable commits on your branch by maintainers? I have a few tiny tweaks I want to make before merging. But this is like 99% of the way there!

@MikeRayUX
Copy link
Contributor Author

"Allow edits by maintainers" box was already checked in the sidebar of the PR.

Is there something else I need to enable somewhere?

@joemasilotti
Copy link
Owner

Weird! No, that should be it.

I'll just add my comments in a review real quick.

@MikeRayUX
Copy link
Contributor Author

I'll just add my comments in a review real quick.

Ok, ready to implement any changes.

joemasilotti
joemasilotti requested changes Mar 23, 2023
View reviewed changes
Copy link
Owner

@joemasilotti joemasilotti left a comment

Choose a reason for hiding this comment

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

Last three changes and we are GOOD TO MERGE! 🎉

test/models/concerns/businesses/hiring_invoice_requests/notifications_test.rb Outdated Show resolved Hide resolved
db/schema.rb Outdated Show resolved Hide resolved
test/fixtures/businesses/hiring_invoice_requests.yml Outdated Show resolved Hide resolved
MikeRayUX and others added 3 commits March 22, 2023 20:12
@MikeRayUX @joemasilotti
Update test/models/concerns/businesses/hiring_invoice_requests/notifi…
9e31a59 
…cations_test.rb

Co-authored-by: Joe Masilotti <joe@masilotti.com>
@MikeRayUX @joemasilotti
Update db/schema.rb
fd37046 
Co-authored-by: Joe Masilotti <joe@masilotti.com>
@MikeRayUX
remove hiring_invoice_requests fixture
e78b3ce
@joemasilotti joemasilotti merged commit 6229bb1 into joemasilotti:main Mar 23, 2023
4 checks passed
@MikeRayUX MikeRayUX deleted the new_hire_form_flow branch March 23, 2023 03:30
@joemasilotti
Copy link
Owner

Merged! Thanks for taking this over the finish line @MikeRayUX. And thank you @RomanTurner for kicking this off an getting us some solid foundation.

Can both of you please email me how you'd like to receive your bounty? I can do GitHub Sponsorships, Venmo, Zelle, or PayPal.

And @MikeRayUX, whenever you have a minute can you please document the next steps for the refactor? Feel free to assign yourself if you want to take on the work.

@MikeRayUX
Copy link
Contributor Author

Thanks again @RomanTurner for your work in this PR!

It was a pleasure working with you on this Joe!

Happy to file an issue and dive in on the next refactor for Hired::Form -> Developers::CelebrationPackageRequest

Will get on that first thing in the morning.

@MikeRayUX MikeRayUX mentioned this pull request Mar 23, 2023
Closed
@joemasilotti joemasilotti mentioned this pull request Mar 23, 2023
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joemasilotti joemasilotti joemasilotti approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@MikeRayUX @joemasilotti @RomanTurner