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 LifeCycle to Project::LifeCycleStepDefinition and co #17248

Open
wants to merge 3 commits into
base: feature/58159-fixed-set-of-project-stages-and-gates-editable-on-project-overview-page
Choose a base branch
from

Conversation

dombesz
Copy link
Contributor

@dombesz dombesz commented Nov 21, 2024

Ticket

Part of https://community.openproject.org/wp/59287

What are you trying to accomplish?

Apply the class renamings discussed here: #17119 (comment) .

Merge checklist

  • Added/updated tests
  • Added/updated documentation in Lookbook (patterns, previews, etc)
  • Tested major browsers (Chrome, Firefox, Edge, ...)

…t::StageDefinition and Gate to Project::GateDefinition
Copy link
Contributor

@toy toy left a comment

Choose a reason for hiding this comment

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

Nice, just few notes. Git rename detection for this PR makes files changed very confusing

@@ -1,5 +1,8 @@
class AddLifeCycleToWorkPackages < ActiveRecord::Migration[7.1]
def change
add_reference :work_packages, :project_life_cycle, foreign_key: true, null: true
add_reference :work_packages,
:life_cycle_step,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think project_ life_cycle_step should be more clear, though lengthier
If you change it foreign_key can be just true

t.string :type
t.string :name
t.integer :position, default: 1, null: true
Copy link
Contributor

Choose a reason for hiding this comment

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

For ordering wherever they are listed?

t.date :start_date
t.date :end_date
t.boolean :active, default: false, null: false
t.integer :position, default: 1, null: true
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is for ordering when listing, then I think it should either follow the order of definitions or use start_date

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants