Skip to content

Conversation

natacha-beck
Copy link
Contributor

No description provided.

@natacha-beck natacha-beck self-assigned this Sep 22, 2025
@natacha-beck natacha-beck added Enhancement Priority: Normal Admin Features or bugs related to administrative features labels Sep 22, 2025
@natacha-beck natacha-beck marked this pull request as draft September 22, 2025 12:04
@natacha-beck natacha-beck marked this pull request as ready for review September 26, 2025 13:25
@natacha-beck
Copy link
Contributor Author

It is part of #841

Copy link
Member

@prioux prioux left a comment

Choose a reason for hiding this comment

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

Check comments.

grouped_options.delete(nil) # data providers that can not be on this list return a category name of nil, so we remove them
grouped_options.keys.sort.map { |type| [ type, grouped_options[type].sort ] }

return grouped_options || []
Copy link
Member

Choose a reason for hiding this comment

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

Why was this return added? Didn't the previous code return an empty array already?

return grouped_options || []
end

def get_personal_type_list #:nodoc:
Copy link
Member

Choose a reason for hiding this comment

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

What does this method do? Explain please.

Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment block before the method name

Copy link
Member

Choose a reason for hiding this comment

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

Why was this file renamed? Since it doesn't include the .erb extension, the tempalate substitutions in it won't work anymore, right?

Copy link
Member

@prioux prioux left a comment

Choose a reason for hiding this comment

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

See my new comments.

grouped_options.keys.sort.map { |type| [ type, grouped_options[type].sort ] }

return grouped_options || []
return grouped_options
Copy link
Member

Choose a reason for hiding this comment

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

This is still wrong. It doesn't do what the original code did, which was to return the value computed by the method chain at line 831. Basically, the list you are returning is no longer sorted.

return grouped_options || []
end

def get_personal_type_list #:nodoc:
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment block before the method name

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

Labels

Admin Features or bugs related to administrative features Enhancement Priority: Normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants