Skip to content

Conversation

@ddey2
Copy link
Member

@ddey2 ddey2 commented Apr 24, 2024

Addressed:

  1. Adding a page for listing all the extractors
  2. Only admins should be able to enable/disable them.
  3. Regular users should see the enabled extractors only.

To test:

  1. run the test_extractors
  2. Try to switch to admin_mode and then enable/disable the extractors
  3. Then try to switch back to regular mode and you should see only enabled extractors.
  4. Try to disable an extractor, then try to submit a job being a regular user from backend(Postman), you should see Forbidden error.

Adding some screenshots for reference.
1
2
3
4
Screenshot 2024-05-02 at 5 16 16 PM
Screenshot 2024-05-06 at 3 38 38 PM

@ddey2 ddey2 requested review from lmarini and tcnichol April 24, 2024 00:59
@ddey2 ddey2 marked this pull request as draft April 24, 2024 00:59
@ddey2 ddey2 marked this pull request as ready for review May 2, 2024 23:06
@ddey2 ddey2 linked an issue May 2, 2024 that may be closed by this pull request
Copy link
Contributor

@tcnichol tcnichol left a comment

Choose a reason for hiding this comment

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

Tested and everything works. Marking approved.

@ddey2 ddey2 requested a review from tcnichol May 6, 2024 20:41
@Vismayak
Copy link
Contributor

Vismayak commented May 7, 2024

Tested and working, approved!

Copy link
Member

@lmarini lmarini left a comment

Choose a reason for hiding this comment

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

Besides some minor comments there is a couple of high level issues I noticed:

  1. Anyone can click on the extractor link, but if you are not in admin mode, nothing shows in the list. Maybe we can have a message for now. Or not show the link in the menu unless you are in admin. I am not sure.
  2. Once I went into admin, I clicked on two extractors, but when I go to the dataset submission page, three show active. I countinued to click and the lists on dataset page and file page of enabled extractors are off from the main page.

Thanks!

@ddey2
Copy link
Member Author

ddey2 commented May 13, 2024

Besides some minor comments there is a couple of high level issues I noticed:

  1. Anyone can click on the extractor link, but if you are not in admin mode, nothing shows in the list. Maybe we can have a message for now. Or not show the link in the menu unless you are in admin. I am not sure.
  2. Once I went into admin, I clicked on two extractors, but when I go to the dataset submission page, three show active. I countinued to click and the lists on dataset page and file page of enabled extractors are off from the main page.

Thanks!

Hi Luigi,

  1. I agree. I was thinking of hiding the link in the menu unless you are an admin and not in admin_mode.
  2. I am not able to reproduce that. I am trying with 2 extractors. It reflects only the enabled extractors on my file/dataset page.

@ddey2 ddey2 requested a review from lmarini May 13, 2024 20:58
@ddey2
Copy link
Member Author

ddey2 commented May 16, 2024

May 16th meeting -

  1. try toggle button instead of checkbox
  2. List all the extractors, disable toggle button for nonadmin mode
  3. make set_active_flag method as private and let other endpoints call them

@ddey2
Copy link
Member Author

ddey2 commented May 16, 2024

Major update:

  1. Use toggle switch button instead of checkbox
  2. Showing tooltip for disabled toggle button

For latest UI look, please refer to ss

Screenshot 2024-05-16 at 2 05 16 PM Screenshot 2024-05-16 at 12 23 04 PM

@ddey2 ddey2 requested a review from longshuicy May 16, 2024 19:17
Copy link
Member

@lmarini lmarini left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks for making those changes.

@lmarini lmarini merged commit 875dfe8 into main May 21, 2024
@lmarini lmarini deleted the 1009-list-all-the-extractors-and-also-add-ability-to-turn-onoff-the-extractors branch May 21, 2024 21:18
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.

List all the extractors available to a dataset List all the extractors and also add ability to turn on/off the extractors

6 participants