Add function to display all components in a complex object. #6

[VivianChu]

Update the format

[VivianChu]

@ucsdlib/developers - Please review, comment, etc... Thanks

[hweng]

@mcritchlow @VivianChu I reviewed the code and compared its tests to curation_concerns factory_girl tests, which is very similar. Looks fine to me.

[hweng]

And I will approve it after demo.

[VivianChu]

Thanks @mcritchlow and @hweng for reviewing.

[mcritchlow]

@lsitu if you get a chance, since you're back, would you mind taking a look too? This is what @VivianChu is going to demo in this morning's mtg.

[VivianChu]

Thanks everyone for reviewing. Please hold off the merge because I need to update the code to display the component if there's no attached file.

[VivianChu]

@ucsdlib/developers - please review again my new commit https://github.com/ucsdlib/dams5-cc-pilot/pull/28/commits/578c1bb2c41a66e800e0698e682517c3157e004d I will squash them into a single commit before anyone merge them. Thanks.

[mcritchlow]

@VivianChu we could actually test the Squash and Merge feature instead if you'd like.

[VivianChu]

@mcritchlow - Yes, Squash and Merge feature sounds good

[mcritchlow]

@hweng or @lsitu does the recent commit look OK to you? If so, I think we can get this merged.

[lsitu]

@mcritchlow I think it looks good over all but it'll be nice if the if condition logic can be switched.

[mcritchlow]

@lsitu 👍 please feel free to recommend changes inline.

[VivianChu]

@lsitu Could you recommend changes about the condition logic so I could try it out? Thanks

[lsitu]

@VivianChu : Here it is: https://github.com/ucsdlib/dams5-cc-pilot/pull/28/commits/578c1bb2c41a66e800e0698e682517c3157e004d

I think it's a little bit cleaner and avoid missing components again when values.member_presenters is not presented. But I haven't tested it yet. Thanks.

[VivianChu]

@lsitu I only see my commit. I don't see your suggestion there.

[lsitu]

@VivianChu Sorry I see my comments are marked as pending and I just submitted it. Can you see it now?

[VivianChu]

@lsitu I tried that and I got error undefined method `member_presenters' for #CurationConcerns::FileSetPresenter:0x007fb31708bf20 Before when I tested different case, I already tried to switch the statement, and got that error before.

[lsitu]

@VivianChu I see. Maybe we can just figure out a way to fix that gap with missing components in the else statement when alues.member_presenters is not presented then? How about something like: if value.respond_to? :member_presenters && values.member_presenters.present? else end

[VivianChu]

@lsitu For that change, the error is gone. However, the component that has no attached file is missing.

[lsitu]

@VivianChu Yes. I think the gap is still there and I've just updated my previous comment with the if condition to: if value.respond_to? :member_presenters && values.member_presenters.present?

Will it work?

[VivianChu]

@lsitu I got this error "false is not a symbol nor a string." def component(values, type, results = [], level) if values.respond_to? :member_presenters && values.member_presenters.present?

I tried it before and got the same error.

[VivianChu]

@lsitu I think we could just leave the logic as it is now and go back to refactor, try different logic when we have time later. There's always room for improvement. What do you think?

[lsitu]

@VivianChu Okay.

[VivianChu]

Thanks everyone for reviewing and @lsitu for merging.