Ruby/Rails package updates; code maintenance and remove dependency of legacy cerberus_core package

[lukehollis]

• https://trello.com/c/VQ4BaQz3/162-update-rails-ruby-versions-and-remove-cerberus-hydra-fedora
• https://trello.com/c/PZlyecz0/192-ensure-all-routes-and-templates-render-correctly
• https://trello.com/c/06JyrPn5/196-restore-content-backups-to-dev-site
• https://trello.com/c/HVqcWAU6/194-ensure-authentication-system-works-correctly-with-ruby-updates

[amclark42]

Hello, @lukehollis! I'm trying to get the new code running on my laptop for testing, but I'm having no luck. (Admittedly, I am a Rails novice.) Could you add some documentation on installation, perhaps to the README?

Are there any configuration changes or new system assumptions for the production and staging environments?

[lukehollis]

Hi all, this PR includes the following now as well:

• https://trello.com/c/ibGKLEKi/189-replace-xml-file-upload-handling-in-fedora-with-something-more-widely-maintained
• https://trello.com/c/raxZcgwV/152-creating-collections-in-projects
• https://trello.com/c/rqfw7y70/181-tei-files-disappearing
• https://trello.com/c/Bld4yXy9/180-debug-ghost-records-of-tei-files
• https://trello.com/c/wTartxLW/195-update-tests-to-pass-with-rails-versions-upgrades
• https://trello.com/c/SbRmdjcJ/193-reinstate-startup-initializers

[amclark42]

2ec1c81 was already reviewed

59e6957 upgrades Ruby and a lot of gems. There are a lot of changes; presumably these were in

[amclark42]

065cd2f : The removal of bootsy makes sense, as it’s no longer maintained. However, it was a WYSIWYG associated with News Items and Pages, replacing similar features in Drupal. TAPAS users don’t have currently access to this functionality; it seems safe to just use text fields that can accept HTML.

[amclark42]

59190ce : Comments out search_params_logic in collections_controller (and communities, core_files, users). This means that a Solr instance would have no knowledge of Core Files’ permissions, nor trickle-down permissions from Communities and Collections to Core Files. Permissions handling will likely need to be added back with search indexes.

[amclark42]

75a6f46 : TODO item and commented-out code that may be important down the line? We may have to revisit this.

app/controllers/core_files_controller.rb, line 13 – 14:

# TODO investigate this after ruby version upgrades complete
# skip_before_action :load_asset, :load_datastream, :authorize_download!

[amclark42]

f05ec11 : Uncomments code that makes view packages info into the TAPAS home page. We already knew we’d have to change that, but this is where it happened.

In users_controller, there’s a comment about a Rails deprecated helper, but the new return value is just the root path instead of a revised bit of code.

[amclark42]

69d06558 : Fairly straightforward, but we should make sure that the character sets match TAPAS users’ needs. Ideally an international encoding standard like UTF-8 should be used for any table with fields that users can edit.

[amclark42]

fa1db84 looks straightforward. We’re not sure why the five_* methods exist. :community_members -> memberships?

aa8a4cd is good.

90e2de4: What are :community_communities and :collection_collections, and parent communities/collections? What are the indexes supporting?

[amclark42]

19600da Puzzling over:

Return the collection where we store TEI files that reference non-existant collections. If it doesn't exist create it.

Collection records created in Rails because Drupal didn’t explicitly tell Rails to create them...?

2cfb33c:

• Move queries out of the user controller! They’re not terrible where they are, but not ideal.
• MODS fields were commented out of the Core File model; we’ll probably need to revisit where and how to add back metadata.
• Does Core File replace Record?

26223ad removes Fedora and Solr queries.

[amclark42]

d4779e1 Why use bigint rather than integer in column datatypes?

7ebc714 Introduces CanCan(Can?) gem for authorization.

7d9bab7 looks good. When do users get thumbnails?

5f9b9cf Institutions don’t need to be models, they’re simple strings, metadata. Nothing is done with an Institution-as-entity. Institutions may have been added to confer TEI membership. Since TAPAS will be free to all users, we definitely don’t need this.

[amclark42]

4e7987d is good.

743e5aa seems to be mostly a condensed version of previous commits:

• models/concerns/did.rb removes attributes for the Drupal identifier and MODS metadata.
• We’re still not sure where the parent communities are coming from.
• SQL tables are mostly using the latin1 character set, or utf8mb4. We’ll need UTF-8 at least for internationalization.
  • Based on a file auto-added by Sunspot, this might be a default that went unexamined.
• IDs are set as integer datatypes.
• We think some of the gems ultimately aren’t used, but are still listed in the Gemfile by the final commit. We’ll need to check later.

9e11682 is good.

0dd29be:

• Still confused about why communities can have_many other communities.
• Is a community “depositor” the person who creates the community? Why would a community belong to someone who deposits a core file?

[amclark42]

0440da0 looks good

d88a5c3: The database schema adds a limit on the number of characters allowed in the CSS and JS text fields for view packages. Worth keeping an eye on, since TAPAS users don't control view packages; we do.

fbdbe58 Seems to introduce a regression in the database schema — things that were changed in the last commit get changed back. In the Community spec, the test for "as_json" is commented out. We're wondering if that method was intended to pass data to Drupal, in which case maybe we don't need it at all. The User spec has a test that doesn't do anything, and a typo. We're assuming that gets fixed later.

[amclark42]

d2c9f6b looks good

ae2c899 looks good

8c87f9d is overall good. I’m a little concerned about taking logic out of the HTML templates for search results. The empty <div>s are there, possibly so that a JS asynchronous call can populate it. But Rails has the data and can start populating the page; JS can take over and make the page of results responsive. Rails is built for filling in templates and making websites; why not make use of its capabilities?

[amclark42]

2aee5dc The mix of HAML and ERB is irritating. It’s hard to switch from reading one to the other. Otherwise good.

e65e15c looks good.

a004414 has joins and wheres in the collection controller. In the collection model, it looks like the collection is the owner of many thumbnails?

[amclark42]

a004414 What did mass_permissions accomplish in the older code? It was replaced with is_public, but we don’t know what the old system was.

Also, we’ll need to revisit the data_form template. A community/project setting of “private” should cascade down to collections. A collection in private community should have its visibility set to private, and the form’s checkbox should be disabled, with an explanatory note on why the collection cannot be made public.

7ac98fe looks good; see last paragraph on future revisions to the form

[amclark42]

38fbe8a:

• The depositor of a collection is a user. Does the depositor have special permissions? What happens if the associated user is deleted?
  • We finally see the depositor represented in the schema and a migration for collections!
• In the collections data form, the name attributes of form fields are deleted. Those should be added back so that the form can be sent.
• An answer to our question on why a collection can have many “collections”: So-called “collections” are the records, the Core Files, belonging to a collection. The records are the “collections” of their containing entity. It’s really confusing, and a poor renaming choice.

[amclark42]

7fb651c The code that confused Charles was actually added by Archimedes. It seems to be resolved in later commits, though! Collections will belong to only one community.

a5eed45 looks good.

4f1abf9 Users become owners of Core Files, a relationship that likely became necessary because Drupal had taken on that logic.

[amclark42]

0b8f200 looks good.

78713de

• In app/controllers/core_files_controller.rb, there's an incomplete list of file types (the only one is "TEI Record"). The others may be 'ographies, images, thumbnails? We'll have to check progress on this, possibly reimplement.
• What is the "placeholder user" referenced in the commit message?
• As noted previously, a "private" setting on the community and collection should cascade down to core files.

7da7bac The index method in the users controller has code that needs to be moved elsewhere for security. This commit is preparing to make Users as authors and contributors to a file, but those fields are just supposed to be text fields.

9d3c693 This is where core files' authors and contributors become user entities instead of strings.

[amclark42]

b79bd06 removes code duplicated from the Core File upsert method. We may still want to kick off a MODS generation process on TEI upload.

8baa16b looks good

5c261a4 looks good

3afc726 is a start at documentation.

8229892 removes some of the logic originally in the Core File model method project. May have to beef that up.

[amclark42]

296c9ff

• A fix: collections have_many thumbnails, the as: owner has been taken out.
• The model for Thumbnail has been removed, but references to it still persist.
• collection.collections -> collection.core_files, yay!
• AWS is added as storage for files in the dev and production environments. File storage is used for test.
• Captions table is added. Something they tried that didn’t work out?

ce82bbe looks good.

[amclark42]

8a5cdca adds and streamlines the destroy methods in the community and collection controllers. When a community is deleted, its collections and core files should be deleted. Not sure this is represented implicitly, it doesn't seem to be represented explicitly in most current code. For collections, their core files should only be deleted if they have no other associations with still-existing collections. Tests would help here! We should dig more into this.

d4ad8bf is good

06b2223:

• The create_members method on the community model should explicitly determine the user's role. We can define the undefined behavior. Tests are needed here.
• Need to revisit the data form for selecting members of communities. Reverse the order of roles (Admin, Editor, Member), and make sure that Can Can (Can?) reflects the fact that admin permissions encompass editor and member permissions, etc. Add logic back so that a user is assigned a role once, not in subsequent role selects.
• Not sure about the use of to_json in the template for showing communities. It might be to aid the readability of pm[:roles][0]?

[amclark42]

38b4861 adds default_scope to the community model, which is apparently awful practice. But the most recent code removes it, so, good! The to_json thing from the last commit was removed.

e299e3b Make sure we're modifying redirect_to ... and return, the return bit is unneccessary. The Core File "thumbnails" are likely associated image files; rename?

ac7eb98:

• Maybe investigate if the CanCanCan initialize method has to set permissions separately for community members versus depositors. (Is the depositor not a community member?)
• Lots of commented-out code in the Collection model is deleted, including a method call that would update core files after the collection (cascading permissions would be applied here, probably). We'd want to add that back, and reimplement MODS metadata, 'ographies.
• We can remove the paid and unpaid user distinction in the User model.
• Still want name attributes on form elements.
• We should review the acceptable file extensions for uploads, incl. thumbnails.
• Do we want to keep the admin_at column on the user model? It could be a boolean value instead of a datetime one. What does the latter buy us?

[amclark42]

Comments re: #82:

• It looks like users are required to have an institution. (See add_institution in the communities controller.) We don't need users to provide anything like that information, so we should remove institutions as an entity or put in a test that adds institutional data if provided.
• In the communities controller, add_members should allow empty fields for project members and editors. Later: It looks like those fields are optional, but the add_members method still needs some cleanup.
  • We should revisit and make a decision about how to allow people to add other users to projects.
  • Before a community created, grab the user id and auto-fill them as the admin. This should be part of the create method rather than add_members.
• Need to set the thumbnail file types to something sane, with helpful form labels and explaining text
• We noticed that there's a discard, or a soft delete, for getting rid of a community. This is more like archiving the community rather than deleting the entity. We should work through what should happen when someone wants to delete a community.
• Core files controller has been trimmed down to use CanCan's accessible_by(current_ability) instead of a SQL query.
• In the core files controller, new, created, and edited core files do not have modifiable types. It looks like 'ographies can't be specified as such anymore. This was code that was previously marked as "FIXME: (charles) What is this supposed to do?"—now, it's just been deleted.
  • The Core File model still has references to 'ographies and flavors of TEI file.
  • The data form previously commented out references to 'ographies (again, because of confusion about they were), so people wouldn't be able to adjust the type of file.
  • It looks like the deleted code in new was some kind of hack or test code, so fair enough. The user still needs to be able to designate 'ographies when creating and editing the core file.
• We need .kept in files.collections.kept.first because of soft-deleted collections. Will need to revisit this logic.
• What's the logic behind a "featured" core file? What happens when an admin designates this? If a "featured" core file is in multiple collections, is it somehow prioritized in each one?

[amclark42]

Back to #82:

• In the users controller, a delete endpoint and method is added. Admins can discard, then delete users. Might want to formalize this later.
  • There's not yet any view to allow admins to delete users.
  • Rename five_* methods to something like preview_*
  • Revisit those methods and move abstracted SQL queries to User model.
• In the community model, a community can only have one thumbnail, which makes sense.
  • Deleted methods to create/edit members. It looks like membership is stored with Users (through Community Members?), and the Community now makes queries there to find the members associated with it
  • project_members has been rewritten to get only the users with member type of "member", rather than all users associated with the community. We are now missing a method to see all users associated with a community, regardless of role.
  • When we create tests, the necessary logic here might become clearer.
• In the Core File model, we have a User type "depositor" that might not appear anywhere else. Look further into this later.
  • Noticing that user_type refers to users' relationship to core files, and member_type is the relationship with projects/collections?
  • Deletes a lot of 'ography and metadata definitions. :(
  • The previous association with Collections has been deleted in favor of creating a definition for assigning the core file to 1+ collections.
  • At this point, the relationship between core files and collections feels incomplete and under-theorized. Tests would help.
• UsersCoreFile has been renamed to CoreFilesUser
• In the User model, the ROLES constant (e.g. for site admin) has been deleted
  • Not sure what forem_name is. Edit: This is a gem that was previously commented out.
• In app/views/communities/_data_form.html.haml, the name is removed from the label and field for thumbnail. This seems like it would remove attributes linking the two, making the form inaccessible for screen readers. We need to add name: "thumbnail" back in both places.

Left off with the core file data form.

[amclark42]

Continuing with #82:

• In app/views/core_files/_data_form.html.haml, we were initially confused about why there are references to both Users and Core Files' Authors/Contributors. It does make some sense and we're not too worried about it.
  • In discussing thumbnails versus support files, we realized that the push here to allow more than one thumbnail might have been due to a misunderstanding of the role that images play in TEI files. A core file might have a thumbnail to represent itself, but it might also have images in the "support file" category ("auxiliary file" in production), which get shown as part of the HTML display. At one point I remember that we communicated a desire to associate a file with more than one image. Archimedes may have taken that to mean more than one thumbnail.
• In the future, we should revisit the forms for accessibility, making sure the labels map to their form components, etc.
• config/routes.rb adds destroy to users
• The migrations include columns to enable the soft deletion of users and core files.

[amclark42]

Last part of #82!

A couple weird things about the users index view:

• The only reason to have a index of users is for administrators to review and manage them. The conditional should be moved up so only admins can see this page.
  • Actually, @candyhazlett uses the Admin Dashboard for this. We don't need a route for a Users index.

[amclark42]

Starting #83

• The devise_invitable gem is added.
• They're starting to remove Blacklight and search in the controllers.
• Admins can invite users
• We noticed that users don't have to include a name, so pages address them by email
  • The schema doesn't require a name, but the new invitation view does have name as a required field
• Deleted some methods, since Solr isn't needed to identify a user's 5 communities/collections/records
• apps/views/catalog/_search_results.html.erb expands the page a good deal, but adds an "Actions"—if the user can manage the resource, they have the option of editing or deleting the item. We think it's strange to put that in search results.

[amclark42]

Continuing #83 :

• We are confused by the presence of app/views/users/invitations/edit.html.erb and the new invitation form. We don't think they're being used and the forms haven't been touched on the Archimedes develop branch since the initial commit.
  • We aren't against having user invitations, but we wish we understood how it was intended to work in the Rails ecosystem.
  • The users route now has a controller for invitations and Admins can create users (via invites?)

[amclark42]

In commit 26db5c1, Solr configuration and gems are removed, as other gems for faceted search are added. Because we wanted to keep Solr (and eXist, and the view package system), we are going to stop merging Archimedes' commits after c1b8b02.

Skimming the rest of the commits in the PR, here's what we're choosing to skip out on:

• Faceting and search made to work using the different gems
• Converting the view package XSLTs into ERBs for SaxonJS transformation
• Complete removal of Blacklight
• Complete removal of eXist
• Drupal migration scripts
• Some changes to code tests (they didn't do much)
• Some documentation and comments provided after the fact

[amclark42]

Reviewing c1b8b02, we decided we don't actually want to merge it in either. It makes the Core File controller inherit from the Application controller instead of the Catalog controller, but if we're keeping Solr and Blacklight (and we are!) then we are just fine with keeping the original inheritance.

FINIS.