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

Add support for gem consumer custom templates #39

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

jbanass
Copy link
Contributor

@jbanass jbanass commented May 10, 2024

  • Add support for gem consumer custom templates

  • Update changelog, docs

  • Fix indentation for multiline templates

  • Resolve rubocop violations

  • Use underscore for custom template path

This PR aims to merge in the fix for #38 into the upstream.

This allows consumers of Onesie to create templates using generators to quickly and reliably perform repeating Onesie tasks (like adding a new Permission, feature access, etc.), and allows documentation for performing these types of works to sit next to the code performing it.

Example usage:

$ bundle exec rake onesie:new_template['NewPermission']
      create  onesie/templates/new_permission.rb

Resulting file:

# Write the contents of Onesie::Tasks#run here!

Can be updated to be:

# This template serves as an "API" to easily create permissions with Onesies
# If there are any questions or concerns, please reach out to
# #guardians-of-the-velocity and reference the ticket number for help

# Select which actions are applicable for the new permissions
applicable_actions = %w[
  create
  read
  update
  delete
]

# Update the resource to be the resource we're adding (for ex: users)
resource = 'REPLACE ME'

# Select which applicable access contexts are for this permission
access_contexts = %w[
  Tenant
  Branch
  Group
]

# Select which role slugs we should add the new permission(s) to
roles_to_add_permissions_to = %w[
  admin
  internal
]

new_permission_slugs = applicable_actions.map do |action|
  puts "Creating #{action}:#{resource}".cyan
  Permission.where(
    resource: resource,
    access_contexts: access_contexts,
    action: action
  ).first_or_create!
end.map(&:slug)

roles = Role.where(slug: roles_to_add_permissions_to)

roles.each do |role|
  puts "Adding #{ permission_slugs} to #{role.slug}".cyan
  role.add_permission_slugs(new_permission_slugs)
end

Then, when we run:

$ bundle exec rake onesie:new['DEV-1234-add-new-foo-permission',,'NewPermission']

we get:

# frozen_string_literal: true

module Onesie
  module Tasks
    class Dev1234AddNewFooPermission < Onesie::Task
      allowed_environments :all
      manual_task enabled: false

      def run
        # This template serves as an "API" to easily create permissions with Onesies
        # If there are any questions or concerns, please reach out to
        # #guardians-of-the-velocity and reference the ticket number for help

        # Select which actions are applicable for the new permissions
        applicable_actions = %w[
          create
          read
          update
          delete
        ]

        # Update the resource to be the resource we're adding (for ex: users)
        resource = 'REPLACE ME'

        # Select which applicable access contexts are for this permission
        access_contexts = %w[
          Tenant
          Branch
          Group
        ]

        # Select which role slugs we should add the new permission(s) to
        roles_to_add_permissions_to = %w[
          admin
          internal
        ]

        new_permission_slugs = applicable_actions.map do |action|
          puts "Creating #{action}:#{resource}".cyan
          Permission.where(
            resource: resource,
            access_contexts: access_contexts,
            action: action
          ).first_or_create!
        end.map(&:slug)

        roles = Role.where(slug: roles_to_add_permissions_to)

        roles.each do |role|
          puts "Adding #{ permission_slugs} to #{role.slug}".cyan
          role.add_permission_slugs(new_permission_slugs)
        end
      end
    end
  end
end

Please ensure the following:

  • The PR relates to only one subject with a clear title and description
  • The changes are reflected in the CHANGELOG in the unreleased section
  • Reference the related issue if one exists, Fix #[issue number]

* Add support for gem consumer custom templates

* Update changelog, docs

* Fix indentation for multiline templates

* Resolve rubocop violations

* Use underscore for custom template path
@jbanass
Copy link
Contributor Author

jbanass commented May 10, 2024

@timlkelly eager to hear your thoughts!

@jbanass
Copy link
Contributor Author

jbanass commented Jul 15, 2024

@timlkelly just wanted to bump so it doesn't fall off the radar. We're using this in our fork, so no rush

Copy link
Owner

@timlkelly timlkelly left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together. Overall it makes sense, there are some edge cases I discovered though which I think need to be resolved.


The new template argument doesn't play nicely with the rails generator.

~/Desktop/onesie-test main ✗ ❯ rails g onesie:task tttask TemplateOne
      create  onesie/tasks/20240721202939_tttask.TemplateOne.rb
~/Desktop/onesie-test main ✗ ❯ rails g onesie:task tttask high TemplateOne
      create  onesie/tasks/20240721202952_tttask.high.rb

Notice that the first command without a priority arg uses the template in the filename.

Also, once this is fixed, I think the usage file could use details on the rails generator.


If you pass in a template that does not exist, it errors but still creates an empty task file:

~/Desktop/onesie-test main ✗ ❯ bundle exec rake onesie:new['DEV-1234-add-new-foo-permission',,'TemplateOneTwo']
rake aborted!
Errno::ENOENT: No such file or directory @ rb_sysopen - /Users/tim/Desktop/onesie-test/onesie/templates/template_one_two.rb (Errno::ENOENT)
/Users/tim/.rbenv/versions/3.3.4/bin/bundle:25:in `load'
/Users/tim/.rbenv/versions/3.3.4/bin/bundle:25:in `<main>'
Tasks: TOP => onesie:new
(See full trace by running task with --trace)

Probably less important but i noticed that if you generate a template with a underscore or hyphen it will create the file with an underscore. but it does not transform a space in the template name to a underscore. Unsure how necessary this guardrail is, but wanted to note.

@@ -7,7 +7,7 @@ class <%= class_name %> < Onesie::Task
manual_task enabled: false

def run
# Write your Onesie Task here
<%= run_contents.chomp.indent(8) %>
Copy link
Owner

Choose a reason for hiding this comment

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

Does this line not work if it is intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you see a template in which consumers of the gem do not want the output to be formatted properly?

Copy link
Owner

Choose a reason for hiding this comment

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

No, it should be formatted correctly. I was wondering why it was required to call the .indent method. I'm assuming it has to do with the template parsing.

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 don't know if required is the right term here. From a formatting perspective, if we don't include the indent, a onesie generated via template will appear as:

# frozen_string_literal: true
module Onesie
  module Tasks
    class <%= class_name %> < Onesie::Task
      allowed_environments :all
      manual_task enabled: false
      def run
template here      
      end
    end
  end
end

Which will either make engineer's eyes twitch (or rubocop 😂 )

module Onesie
module Generators
# Generates a new Onesie template
class TemplateGenerator < Rails::Generators::NamedBase
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add documentation in the how_to_guides/usage.md about this generator please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timlkelly Done!

@timlkelly
Copy link
Owner

@timlkelly just wanted to bump so it doesn't fall off the radar. We're using this in our fork, so no rush

How as it worked for you all so far? I am curious how many templates are in use.

@jbanass
Copy link
Contributor Author

jbanass commented Jul 23, 2024

@timlkelly just wanted to bump so it doesn't fall off the radar. We're using this in our fork, so no rush

How as it worked for you all so far? I am curious how many templates are in use.

It's worked well! Especially given the onesie templates can inline documentation to our internal docs site, it's been useful for providing context behind the template and giving folks the ability to read more about it while performing the task at hand.

We have two existing templates, and one I'm about to open another pull request for.

Comment on lines +23 to +24
[--priority] # Specify a priority that the onesie should run as
[--template] # Specify a custom template for generating the onesie
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timlkelly let me know if this is what you'd expect. I figured since we now accept multiple args, using options is acceptable like this.

module Onesie
module Generators
# Generates a new Onesie template
class TemplateGenerator < Rails::Generators::NamedBase
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timlkelly Done!

Comment on lines +9 to +11
unless File.exist?(path)
puts "Custom Template could not be found at #{path}".red
return
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@timlkelly this is to resolve the file being created despite the template not existing.

I was not able to find a way to prevent the generator to not create the file, even with raising an exception here, so I'm defaulting to an error message being printed, and in the consumer of this, defaulting back to the default template.

@jbanass jbanass requested a review from timlkelly August 25, 2024 17:50
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