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

Rename default pet task to checklist task template #847

Draft
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

ErinClaudio
Copy link
Collaborator

@ErinClaudio ErinClaudio commented Jun 25, 2024

🔗 Issue

(#834)

✍️ Description

📷 Screenshots/Demos

@ErinClaudio
Copy link
Collaborator Author

ErinClaudio commented Jun 25, 2024

@ErinClaudio ErinClaudio force-pushed the Rename-DefaultPetTask-to-ChecklistTaskTemplate branch 2 times, most recently from d2cdc9d to 970335e Compare June 27, 2024 14:30
* add: authenticate_user! as a before action in application controller instead of calling it within multiple controllers

* add: skip_before_action authenticate_user! for controllers that don't need user authentication

* remove: commented out 'skip_before_action' from controllers

* fix failing tests: after adding authentication

* add: config for devise
configuration for setting up custom failures through warden in the devise.rb

* add custom failure file

* update: failure app

 Co-authored-by: Paul DobbinSchmaltz <[email protected]>

* add more context to why this authentication failure app is needed.

* update: rails standard fix
@ErinClaudio ErinClaudio force-pushed the Rename-DefaultPetTask-to-ChecklistTaskTemplate branch from 7d72b16 to 46cf248 Compare July 2, 2024 18:30
* add: authenticate_user! as a before action in application controller instead of calling it within multiple controllers

* add: skip_before_action authenticate_user! for controllers that don't need user authentication

* remove: commented out 'skip_before_action' from controllers

* fix failing tests: after adding authentication

* add: config for devise
configuration for setting up custom failures through warden in the devise.rb

* add custom failure file

* update: failure app

 Co-authored-by: Paul DobbinSchmaltz <[email protected]>

* add more context to why this authentication failure app is needed.

* update: rails standard fix
* add: authenticate_user! as a before action in application controller instead of calling it within multiple controllers

* add: skip_before_action authenticate_user! for controllers that don't need user authentication

* remove: commented out 'skip_before_action' from controllers

* fix failing tests: after adding authentication

* add: config for devise

* add custom failure file

* update: failure app

 Co-authored-by: Paul DobbinSchmaltz <[email protected]>

* add more context to why this authentication failure app is needed.

* update: rails standard fix

Update with failures but no errors
@@ -1,4 +1,4 @@
class CreatePageTexts < ActiveRecord::Migration[7.0]
class CreateCustomPages < ActiveRecord::Migration[7.0]

Choose a reason for hiding this comment

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

Why class name is different than file name? I am not sure if this is good way to go

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes it should be the same. This was fixed in main branch a few days ago as it was an error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you

@kasugaijin
Copy link
Collaborator

@ErinClaudio the errors look like they are related to an issue in main branch a few days ago related to the rename of page texts to custom pages. I pushed a fix to main. There were three migrations renamed incorrectly, each to do with page texts. 7947be3

I’d start there to make sure these changes are reflected in this PR

* add: authenticate_user! as a before action in application controller instead of calling it within multiple controllers

* add: skip_before_action authenticate_user! for controllers that don't need user authentication

* remove: commented out 'skip_before_action' from controllers

* fix failing tests: after adding authentication

* add: config for devise
configuration for setting up custom failures through warden in the devise.rb

* add custom failure file

* update: failure app

 Co-authored-by: Paul DobbinSchmaltz <[email protected]>

* add more context to why this authentication failure app is needed.

* update: rails standard fix
* add: authenticate_user! as a before action in application controller instead of calling it within multiple controllers

* add: skip_before_action authenticate_user! for controllers that don't need user authentication

* remove: commented out 'skip_before_action' from controllers

* fix failing tests: after adding authentication

* add: config for devise
configuration for setting up custom failures through warden in the devise.rb

* add custom failure file

* update: failure app

 Co-authored-by: Paul DobbinSchmaltz <[email protected]>

* add more context to why this authentication failure app is needed.

* update: rails standard fix
* add: authenticate_user! as a before action in application controller instead of calling it within multiple controllers

* add: skip_before_action authenticate_user! for controllers that don't need user authentication

* remove: commented out 'skip_before_action' from controllers

* fix failing tests: after adding authentication

* add: config for devise

* add custom failure file

* update: failure app

 Co-authored-by: Paul DobbinSchmaltz <[email protected]>

* add more context to why this authentication failure app is needed.

* update: rails standard fix

Update with failures but no errors
@kasugaijin
Copy link
Collaborator

kasugaijin commented Jul 18, 2024

Comment for tracking purposes - I have discussed with @ErinClaudio and we will go to MVP with the default pet task architecture and can take our time to build out the Checklist::TaskTemplate, Checklist::TaskItem and Checklist::TaskAssignments from this PR/branch

Tagging @jmilljr24 @mononoken for visibility.

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