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

1815 Handle locked (or not locked) materialized views better functionality #1877

Merged
merged 21 commits into from
Jun 27, 2024

Conversation

Ferrisx4
Copy link
Member

Bug Fix

Issue #1815

Tripal Version: 4

Description

This PR adds or modifies several functionalities related to custom tables and materialized views:

Visual

  1. Make "Populate" the default action on the materialized view list; edit is a dropdown action now.

  2. The locked status of custom tables and materialized views is now shown in their respective listings on admin/tripal/storage/chado/custom_tables and admin/tripal/storage/chado/mviews
    Screen Shot 2024-05-14 at 10 37 38 AM

  3. On the Custom Tables page, add a blurb about what these are and how to find their respective materialized views.

  4. On the Custom Tables and Materialized Views lists, the filter now has a description.
    Screen Shot 2024-05-14 at 10 35 28 AM

Functional

  1. When adding a new materialized view, do not allow the form to submit if the materialized view already exists (uniqueness determined by combination of table name and schema).
  2. Disallow editing to custom tables and their respective materialized view if the table is locked.
  3. A materialized view can now be locked at creation time in the GUI.

Testing?

  • The visual changes introduced in this PR can be tested by navigating to the pages above in item 5.
  • To test functional changes, create a new table by using the GUI, then create a materialized view based on that table.
    • If you set the mview to be locked when you created it, you should find that you can no longer edit it.
    • If you try to re-add the same materialized view (with the same table name and schema), the page should give you a helpful warning message.
    • If you try to edit a materialized view that is locked, the Submit button should be disabled and there should be a status message at the top of the form indicating that it cannot be edited.

@Ferrisx4 Ferrisx4 changed the title 1815 Handle locked (or not locked) materialized views better 1815 Handle locked (or not locked) materialized views better functionality May 14, 2024
@Ferrisx4
Copy link
Member Author

Looks like I somehow forgot to follow the branch naming guide this time, oops! I'll resubmit.

@laceysanderson laceysanderson self-requested a review May 14, 2024 17:59
@dsenalik dsenalik added Tripal 4 Any issue or pull request focused on Tripal 4 Group 2 - Data Storage | Tripal DBX | Chado Any issue relating to biological data storage, Tripal DBX and Chado integration, Materialized Views labels May 15, 2024
Copy link
Member

@spficklin spficklin left a comment

Choose a reason for hiding this comment

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

I think @Ferrisx4 that your code must not all be pushed. I'm not seeing the changes shown in the screenshot. There is no "locked" column in the mviews page, and the action doesn't default to "populate" like it shows.

tripal_chado/src/Form/ChadoMviewForm.php Outdated Show resolved Hide resolved
@Ferrisx4
Copy link
Member Author

Oops, I had word wrap enabled in my editor, didn't realize the length of the comment. Thanks for the quick fix @spficklin

For the display issues, the changes on the Custom Tables and Materialized Views views are not picked up automatically. If you did not reinstall Tripal, they will not appear.

Copy link
Member

@laceysanderson laceysanderson left a comment

Choose a reason for hiding this comment

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

For the display issues, the changes on the Custom Tables and Materialized Views views are not picked up automatically. If you did not reinstall Tripal, they will not appear.

You will want to add an update hook then. It looks like the changes are all config related 🤔 and I don't know off my head how to trigger a rediscover of that so you'll need to do some searching.

@Ferrisx4
Copy link
Member Author

Ok thanks @laceysanderson I will look into it!

@spficklin
Copy link
Member

@Ferrisx4, I know how to trigger a reload of config files. Message me on Slack and I'll show you how.

Copy link
Member

@laceysanderson laceysanderson left a comment

Choose a reason for hiding this comment

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

I did some testing on a docker built on this branch and came across the following:

I get the following error when I edit an unlocked materialized view:
Screenshot 2024-06-22 at 3 27 34 PM

The following worked as expected:

  • locked column was filled as expected
  • default actions change as expected
  • editing a locked materialized view was not allowed
  • editing a locked custom table was not allowed
  • editing an unlocked custom table was successful without error.
  • action buttons worked as expected.

Copy link
Member

@laceysanderson laceysanderson left a comment

Choose a reason for hiding this comment

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

I also build a new docker on 4.x, confirmed that the changes in this PR were not seen, then switched to this branch and cleared the cache. At this point the changed did appear as expected ✅ This shows that the error Stephen first found showing that the config had not been updated has been fixed by the additions to the rebuild command in this PR.

@laceysanderson laceysanderson added the waiting on changes Added on a PR when further review is waiting on the requested changes to be made. label Jun 24, 2024
@Ferrisx4
Copy link
Member Author

Hi @laceysanderson, I believe I've fixed the error you found above. The form['locked'] form element was only available when the form's action was set to "add" and not when it was "edit". When editing, the locked value is now picked up from the database and saved in a 'value' form element to make it available for submission.

Copy link
Member

@laceysanderson laceysanderson left a comment

Choose a reason for hiding this comment

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

Perfect! Thanks, this is ready to merge now 🎉

@laceysanderson laceysanderson merged commit 9f0ef84 into 4.x Jun 27, 2024
14 of 15 checks passed
@laceysanderson laceysanderson deleted the 1815-locked_mview_handling branch June 27, 2024 18:01
@laceysanderson laceysanderson removed the waiting on changes Added on a PR when further review is waiting on the requested changes to be made. label Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group 2 - Data Storage | Tripal DBX | Chado Any issue relating to biological data storage, Tripal DBX and Chado integration, Materialized Views Tripal 4 Any issue or pull request focused on Tripal 4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants