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

Disable the maintenance option when creating an item #1755

Merged

Conversation

crismali
Copy link
Contributor

@crismali crismali commented Nov 14, 2024

What it does

Disables the maintenance option on the admin item form and marks items as in maintenance after creating a ticket.

Why it is important

#1743

UI Change Screenshot

Select dropdown for a new item:
Screenshot 2024-11-14 at 2 18 26 PM

Select dropdown when editing an item:
Screenshot 2024-11-14 at 2 17 56 PM

Expanded select dropdown when editing an item:
Screenshot 2024-11-14 at 2 18 08 PM

An item just before creating a ticket:
Screenshot 2024-11-16 at 11 36 48 AM

An item right after creating a ticket:
Screenshot 2024-11-16 at 11 36 59 AM

Implementation notes

For now I've set it up to only put items into maintenance when creating a ticket. I could see a case for marking items as available once the ticket is resolved, but I can also imagine reasons we might not want that to happen (maybe another ticket was created afterwards or that issue is resolved because it's lost now or something).

Copy link
Member

@jim jim left a comment

Choose a reason for hiding this comment

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

This all looks good so far! Nice handling of conditionally adding that "new ticket" link and excellent tests (as usual 😄).

@crismali crismali force-pushed the michael-1743-require-a-ticket-to-set-an-item-to-in-maintenance branch from f892c35 to df59571 Compare November 16, 2024 17:24
@crismali crismali marked this pull request as ready for review November 16, 2024 17:45
@@ -50,6 +52,11 @@ def category_nav(categories, current_category = nil)
end
end

def status_hint(item)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally I just did this in the template, but erb_lint didn't like me using html_safe there. Standard is okay with it though 🤷

@@ -28,7 +28,7 @@
"devDependencies": {
"concurrently": "^9.1.0",
"markdown-toc": "^1.2.0",
"playwright": "1.46.0",
"playwright": "1.45.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pulled this down a version because this needs to be in sync with the gem, but sometimes dependabot slips an update through

Comment on lines +50 to +53
def update_item_status!(ticket)
status = ticket.retired? ? Item.statuses["retired"] : Item.statuses["maintenance"]
@ticket.item.update!(status:)
end
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there is a workflow where staff would want to create a ticket without putting an item into maintenance? Let's merge this as-is but check in at the next stakeholders about this.

@crismali crismali merged commit 7c5915a into main Nov 19, 2024
9 checks passed
@crismali crismali deleted the michael-1743-require-a-ticket-to-set-an-item-to-in-maintenance branch November 19, 2024 05: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.

2 participants