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

Missing default active workflow leads to spurious Page Not Found error #6978

Open
4 tasks
dmolesUC opened this issue Dec 5, 2024 · 0 comments
Open
4 tasks

Comments

@dmolesUC
Copy link
Contributor

dmolesUC commented Dec 5, 2024

Descriptive summary

If the default active workflow is set to a workflow that doesn't exist, creating a new AdminSet produces a 404 error page.

Steps to reproduce the behavior in User Interface (UI)

In a Hyrax 5.0.1 / koppie system:

  1. Remove default_workflow.json from config/workflows.
  2. Create a new Administrative Set.

Actual behavior (include screenshots if available)

  • Administrative Set is created
    • but possibly without the associated PermissionTemplates / Workflows?
  • AdminSetCreateService tries to activate a nonexistent default workflow, raising ActiveRecord::RecordNotFound
  • Default RecordNotFound rescue behavior produces a 404; browser displays “The page you were looking for doesn't exist”

Stack trace:

  Sipity::Workflow Load (0.8ms)  SELECT "sipity_workflows".* FROM "sipity_workflows" WHERE "sipity_workflows"."permission_template_id" = $1 AND "sipity_workflows"."name" = $2 LIMIT $3  [["permission_template_id", 573], ["name", "default"], ["LIMIT", 1]]
  TRANSACTION (0.8ms)  ROLLBACK
Completed 404 Not Found in 6312ms (ActiveRecord: 4528.3ms | Allocations: 368059)
  
ActiveRecord::RecordNotFound (Couldn't find Sipity::Workflow):
  
activerecord (6.1.7.8) lib/active_record/core.rb:399:in `find_by!'
hyrax (5.0.1) app/models/sipity/workflow.rb:55:in `activate!'
hyrax (5.0.1) app/services/hyrax/admin_set_create_service.rb:210:in `create_workflows_for'
hyrax (5.0.1) app/services/hyrax/admin_set_create_service.rb:188:in `block (2 levels) in create!'
activerecord (6.1.7.8) lib/active_record/connection_adapters/abstract/database_statements.rb:320:in `block in transaction'
activerecord (6.1.7.8) lib/active_record/connection_adapters/abstract/transaction.rb:319:in `block in within_new_transaction'
activesupport (6.1.7.8) lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize'
activesupport (6.1.7.8) lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
activesupport (6.1.7.8) lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
activesupport (6.1.7.8) lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
activesupport (6.1.7.8) lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
activerecord (6.1.7.8) lib/active_record/connection_adapters/abstract/transaction.rb:317:in `within_new_transaction'
activerecord (6.1.7.8) lib/active_record/connection_adapters/abstract/database_statements.rb:320:in `transaction'
activerecord (6.1.7.8) lib/active_record/transactions.rb:209:in `transaction'
hyrax (5.0.1) app/services/hyrax/admin_set_create_service.rb:184:in `block in create!'
…

Acceptance Criteria/Expected Behavior

  • Administrative Set is created
  • PermissionTemplates / Workflows are created / assigned / activated correctly
  • UI proceeds to the edit page for the newly created Administrative Set

and/or

  • Missing default workflow is communicated to the user in some more helpful way

Related work

UCSB issue comet-local#47

Notes

I'm not sure what the correct behavior is here. Some ideas:

  • it could be invalid at startup to set Hyrax.config.default_active_workflow_name to a nonexistent workflow
  • we could have AdminSetCreateService.create_workflows_for verify that the default workflow exists before trying to activate it, and fall back to one that does exist if not
  • we could have Sipity::Workflow.activate! raise something more specific than ActiveRecord::RecordNotFound so we get a 5xx error instead of a 404

Thoughts?

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

No branches or pull requests

1 participant