resolved #151 #267
resolved #151 #267
Conversation
|
@AhmedZaleh could you pull from master? @rohitpaulk this addresses #151, so I'm assigning you for review. |
|
I rebased my commit, please review and merge. |
| def set_sub_id | ||
| current_id = project.issues.count | ||
| update_attribute(:sub_id, current_id) | ||
| end | ||
| end |
There was a problem hiding this comment.
Hi, your rubocop is failing. We use two spaces for indentation. If you are going to use tabs please configure your editor so that it uses 2 spaces as tabs.
|
There's a lot more to do here. You've added a |
| @@ -49,4 +51,9 @@ def sub_id | |||
| def self.find_from_project(project, sub_id) | |||
| Issue.find(project.issue_ids[sub_id.to_i - 1]) | |||
There was a problem hiding this comment.
This should be modified, and the sub_id method should be deleted.
Also, please add validations that check the presence and uniqueness of sub_id.
| Issue.find(project.issue_ids[sub_id.to_i - 1]) | ||
| def set_sub_id | ||
| return if sub_id | ||
| sub_id = project.issues.count |
There was a problem hiding this comment.
Since this is run before saving, we should set it equal to project.issues.count + 1
There was a problem hiding this comment.
Yes, that's right. Thanks a lot :)
|
Please add validations too, we need to make sure that issues aren't saved to the database without a |
|
I know I must validate the |
|
@AhmedZaleh you can use: validates :sub_id, uniqueness: { scope: :project } |
|
@Alwahsh Thanks a lot :) |
|
np :) |
|
This is still marked as |
|
@rohitpaulk I've created some tests that would be enough to test this change I believe in #292... I'd recommend a squash of commits though :) |
I added the sub_id attribute on the issue model instead of calling a lot of queries to retrieve it from the project model.