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

Make Milestones description required, hide title usage #2195

Merged
merged 3 commits into from
Dec 15, 2017

Conversation

bertocq
Copy link
Collaborator

@bertocq bertocq commented Dec 15, 2017

Where

What

Milestone's seem to need a description but not always a title.

How

  • Making description attribute a required field
  • Hiding the title input in the milestone form (but still giving it a valid value to be future-compatible with possible changes around this, and also allow forks to keep this feature in a different way with the less amount of changes)

Screenshots

Admin's Milestone form with title input hidden:

screen shot 2017-12-15 at 16 42 55

Admin's Milestone list with default value for titles

screen shot 2017-12-15 at 16 43 17

Test

Increased to cover description presence validation and fixed to cover title hiding

Deployment

As usual, check warnings section

Warnings

The presence validation of milestone's description shouldn't affect existing milestones for showing but it will for editing (requiring a description to complete the form submission). A rake task to fill with an empty description text doesn't seem like a great option. I think it's on each fork situation to decide what's best: fill with default value, remove the presence validation, or if there are existing milestones that won't be edited ... just do nothing.

@decabeza decabeza merged commit cec48c7 into master Dec 15, 2017
@decabeza decabeza deleted the milestones_improvements branch December 15, 2017 17:47
@bertocq bertocq mentioned this pull request Dec 16, 2017
clairezed pushed a commit to CDJ11/CDJ that referenced this pull request Jun 26, 2018
…ements

Make Milestones description required, hide title usage
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.

None yet

2 participants