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

BundleContext Active Record Model #209

Merged
merged 5 commits into from
Sep 7, 2018
Merged

BundleContext Active Record Model #209

merged 5 commits into from
Sep 7, 2018

Conversation

SaravShah
Copy link
Contributor

@SaravShah SaravShah commented Sep 6, 2018

In the db/migration these are the following attributes I used and why:

form website: https://sul-preassembly-stage.stanford.edu/bundle_context

project_name as a string => instead of just project on the website (thought adding name might be more clear.)

content_structure as an integer => this used to be project_style. In the code project_style is a hash of hashes that contains content_structure and get_druid_from. Get_druid_from is going to be hard coded, and so I thought we could use content_structure as the name here instead of project_style. This will be easier to use it as an enum. So it isn't like this:

 {
      :project_style => {
        :content_structure => [:simple_image, :simple_book, :book_as_image, :book_with_pdf, :file, :smpl]
      }
}

but instead:

{  :content_structure => [:simple_image, :simple_book, :book_as_image, :book_with_pdf, :file, :smpl] }

bundle_dir as a string => this is the same as the form on the website, user input

staging_style_symlink as a boolean => this used to be just staging_style however since it only has 2 options (copy or symlink), and is almost always going to be copy, putting this as a boolean and defaulting to false, seems like a logical solution. Happy to change this to an enum, if that is easier.

content_metadata_creation as an integer => enum, same as the form on the website

in the Enum section in the app/models/bundle_context.rb i had to add a suffix to end of the enum values because of the name space collision between smpl. I found out you can't have two different enum attributes w/ the same enum value.

Please let me know what everyone thinks of the naming I chose, happy to change any of the names.

connects #172

@coveralls
Copy link

Pull Request Test Coverage Report for Build 673

  • 12 of 12 (100.0%) changed or added relevant lines in 4 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.6%) to 67.351%

Totals Coverage Status
Change from base Build 663: 0.6%
Covered Lines: 623
Relevant Lines: 925

💛 - Coveralls

@atz
Copy link
Contributor

atz commented Sep 6, 2018

get_druid_from will not exist at all. The code will always pull druid from the manifest. I agree project_style isn't a very good name.

@atz
Copy link
Contributor

atz commented Sep 6, 2018

Right, you can't have 2 enums w/ the same value because of the dynamic methods like (our old statuses in prescat) obj.ok! wouldn't know which attribute to set.

Copy link
Contributor

@jermnelson jermnelson left a comment

Choose a reason for hiding this comment

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

BundleContext ActiveRecord looks good given my limited AR experience.

@jermnelson jermnelson merged commit ec77685 into master Sep 7, 2018
@jermnelson jermnelson removed the review label Sep 7, 2018
@jermnelson jermnelson deleted the AR-BundleContext branch September 7, 2018 14:05
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.

4 participants