-
Notifications
You must be signed in to change notification settings - Fork 14
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
ROAD-536 Add VersionJump model and admin integration #299
Conversation
@@ -52,6 +53,10 @@ def archived? | |||
status == "archived" | |||
end | |||
|
|||
def locked? | |||
locked_at.present? | |||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can revert this change, but was really simple to add (I updated some code that was using project.locekd_at.nil?
directly)
@@ -0,0 +1,2 @@ | |||
class StoryPolicy < ApplicationPolicy | |||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to create all the policies to be able to use this Pundit feature in the admin panel
<%= link_to resource.new_path, class: "bg-white hover:bg-gray-100 text-gray-800 font-semibold py-2 px-4 border border-gray-400 rounded shadow" do %> | ||
New <span class="hidden md:inline"><%= resource.friendly_name %></span> | ||
<% end %> | ||
<% end %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took this back from the original gem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the original gem? madmin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, sorry, from this file of madmin https://github.com/excid3/madmin/blob/14599a85f0fb687d6ad9bf30ee38db9f8b1abe3c/app/views/madmin/application/index.html.erb#L15
record.to_label | ||
end | ||
|
||
# Uncomment this to customize the default sort column and direction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can/should we get rid of these comments if we aren't customizing the default_sort_column
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was already there commented out in the other resources. Not sure why that was not removed before, so I'd say to note remove it just for this resource (maybe some low-priority story can be about cleaning up these comments, I don't want to change files that I don't need to change for a comment)
@arielj One thing I noticed when testing the admin panel is that there is no way to prevent admins from creating the same version jump multiple times. For example, an admin can create multiple Rails projects that have an initial version of 6.0 and a target version of 6.1. There's also nothing stopping admins from creating a version jump that has more than one jump (think a version jump from 6.0 to 7.1 for example) This might lead to users being confused and possibly disjointed data. I don't know whether we should explore this further, or for our purposes, we can trust our users to not make mistakes. What do you think? |
@torresga I'll add a validation to prevent adding the same jump twice About a jump of more than 1 version I think it's fine? at least I did that intentionally: I was thinking of using, for example, I guess we'll need ernesto to define this better. |
I would leave a small suggestion to add a badge, maybe a small one inside the box, indicating the subproject version jump: |
@arielj Looks good, thank you! |
Jira Ticket
https://ombulabs.atlassian.net/browse/ROAD-536
Motivation / Context
This issue #296
QA / Testing Instructions
Screenshots:
I will abide by the code of conduct.