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

Fix/no_member_validation #1826

Closed
wants to merge 2 commits into from
Closed

Conversation

worksofliam
Copy link
Contributor

Changes

Removes additional check to find out if the chosen member exists. There are two reasons why I think this is allowed:

  1. Because if the user enters a path manually, we can assume it does exist. Even if it doesn't exist, the tab will open with a valid error saying the member cannot be found. Fine by me.
  2. If the user is using the ability to search with the asterisk filter, then we already know that the member exists.
  3. Turns out, on systems with a huge member count this statement was taking a long time to run. That is a problem.

Checklist

  • have tested my change
  • updated relevant documentation
  • Remove any/all console.logs I added
  • eslint is not complaining
  • have added myself to the contributors' list in CONTRIBUTING.md
  • for feature PRs: PR only includes one feature enhancement.

Copy link
Collaborator

@chrjorgensen chrjorgensen left a comment

Choose a reason for hiding this comment

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

@worksofliam The code is in there for a reason: The member extension is not part of the member identification, so the user can enter member.clle and get the source from member.sqlrpgle if not verified - the current code verifies that the extension is correct, so the user will get an error message if it is not the case = better UX.

SQL is not very good at retrieving info about specific members - it lists all info about all members before it selects the one requested. Thus the slower experience...

Can I have a go at this? I have some ideas of how we could retrieve member information much faster than this...

@worksofliam
Copy link
Contributor Author

@chrjorgensen Yes! Be my guest. Perhaps it is also an option to use our checkObject API which has a member attribute now?

@chrjorgensen
Copy link
Collaborator

@worksofliam Unfortunately the checkObjectAPI does not support member extension (uses CHKOBJ behind the scenes), and the member extension is the problem here... but I have other ideas, stay tuned... 😃

@chrjorgensen chrjorgensen self-assigned this Feb 22, 2024
@chrjorgensen
Copy link
Collaborator

@worksofliam New solution is ready - but not using your commits. Should I put it in another PR?

@chrjorgensen
Copy link
Collaborator

PR closed, replaced by #1867.

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.

2 participants