Skip to content

Conversation

@justinlittman
Copy link
Contributor

No description provided.

# TODO: Do we only want to do this for certain updates, e.g., when setting status to completed?
# Enqueue next steps
next_steps = NextStepService.for(step: step)
next_steps.each { |next_step| Queue.enqueue(next_step) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are there some steps that we don't want to enqueue?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, if the template has skipQueue as an attribute I don't think we want to enqueue it. I think NextStepService is only returning things that can be queued though: https://github.com/sul-dlss/workflow-server-rails/blob/master/app/services/next_step_service.rb#L28

@jcoyne jcoyne marked this pull request as ready for review May 3, 2019 13:19
Gemfile Outdated
gem 'dor-services-client', '~> 1.2'
gem 'honeybadger', '~> 4.1'
gem 'jbuilder', '~> 2.5'
gem 'lyber-core', '>= 4.0.3'
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we can inline the one function rather than pull in lyber-core? Lyber-core is another dependency I'd like to kill off.


def current_version
ObjectVersionService.current_version(params[:druid])
# Providing the version as a param is for local testing without needing to run DOR services.
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like us to move to this pattern in production too.

# - RAILS_ENV=development
- SETTINGS__ENABLE_STOMP=false
- SETTINGS__REDIS__HOSTNAME=redis
# image: 'suldlss/workflow-server:latest-dev'
Copy link
Contributor

Choose a reason for hiding this comment

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

remove these commented out lines

@jcoyne jcoyne merged commit 42a4317 into master May 8, 2019
@jcoyne jcoyne deleted the robots branch May 8, 2019 22:07
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.

3 participants