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

BugFix-blocking of blank values insertion on the Credential page. #1064

Closed
wants to merge 2 commits into from

Conversation

astroshim
Copy link
Contributor

What is this PR for?

This PR blocks the blank values insertion on the Credential page and
changes the success message box to zeppelin's dialog box.

What type of PR is it?

Bug Fix

How should this be tested?

Try to save with blank values on the Credential page.

Screenshots (if appropriate)

  • before
    out
  • after
    image

image

Questions:

  • Does the licenses files need update? no
  • Is there breaking changes for older versions? no
  • Does this needs documentation? no

_.isEmpty($scope.credentialPassword.trim())) {
BootstrapDialog.alert({
closable: true,
message: 'The values must be filled.'
Copy link
Member

Choose a reason for hiding this comment

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

How about "Username\Entity can not be empty"?

Also, this will not allow user to have " " whitespace as a password, right? For username that make sense, but I think we should not have such restriction on the password

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bzz Thank you for your review!
I totally agree with you. Let me fix it.

@astroshim
Copy link
Contributor Author

@bzz I fixed as your comments. please review.

@AhyoungRyu
Copy link
Contributor

LGTM 👍

@bzz
Copy link
Member

bzz commented Jun 28, 2016

Thank you!
Looks good to me, mering if there is no further discussion

@astroshim astroshim closed this Jun 29, 2016
@astroshim astroshim reopened this Jun 29, 2016
@asfgit asfgit closed this in 3a338e0 Jun 30, 2016
asfgit pushed a commit that referenced this pull request Jul 11, 2016
### What is this PR for?
Currently, users can add new their credential info for data source authentication in Zeppelin "Credentials" menu. Even though it was saved successfully, they can't see the whole list of credentials in Zeppelin UI.
This PR enables to `get` all credential list, `edit` and `remove` via UI.

*NOTE : Since this patch was implemented based on #1030 API, should be tested after #1030 merged.*

### What type of PR is it?
Improvement & Documentation

### Todos
* [x] - rename `interpreter_authorization.md` -> `datasource_authorization.md`
* [x] - remove `Interpreter Authorization` section (since we don't have this feature yet : [ZEPPELIN-945](https://issues.apache.org/jira/browse/ZEPPELIN-945))
* [x] - rebase after #1030 & #1064 merged
* [ ] - address reviews

### What is the Jira issue?
[ZEPPELIN-1054](https://issues.apache.org/jira/browse/ZEPPELIN-1054)

### How should this be tested?
1. Apply this patch and build `zeppelin-web` as described in [here](https://github.com/apache/zeppelin/tree/master/zeppelin-web#configured-environment).
2. Go to `Credentials` menu.
3. Add new credentials -> you can see the credential info in the credential list table.
4. You can edit & delete them. -> Compare with `conf/credentials.json`

### Screenshots (if appropriate)
- Before
<img width="952" alt="screen shot 2016-06-28 at 12 37 10 am" src="https://cloud.githubusercontent.com/assets/10060731/16407604/69b0c4d8-3cc9-11e6-8284-9abe2969cdc1.png">

- After
![add_credential](https://cloud.githubusercontent.com/assets/10060731/16576765/3671aa16-42cc-11e6-9d9f-dfe1f33f8d37.gif)
If there is no credential
<img width="957" alt="screen shot 2016-06-28 at 12 19 46 am" src="https://cloud.githubusercontent.com/assets/10060731/16407620/7838995e-3cc9-11e6-90ba-1bd0173a1b49.png">

- `datasource_authorization.md`
<img width="845" alt="screen shot 2016-06-28 at 7 58 24 pm" src="https://cloud.githubusercontent.com/assets/10060731/16439169/d4026034-3d6a-11e6-930f-86de12e5fc49.png">
<img width="851" alt="screen shot 2016-06-28 at 7 58 44 pm" src="https://cloud.githubusercontent.com/assets/10060731/16439170/d62f2842-3d6a-11e6-9d3f-ecc5cda29c77.png">
<img width="846" alt="screen shot 2016-06-28 at 8 00 20 pm" src="https://cloud.githubusercontent.com/assets/10060731/16439200/fed58390-3d6a-11e6-9aa2-8cff5a1b7b66.png">

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? yes

Author: AhyoungRyu <fbdkdud93@hanmail.net>

Closes #1100 from AhyoungRyu/ZEPPELIN-1054 and squashes the following commits:

7c38c90 [AhyoungRyu] Fix checkstyle error with jscs rule
ab9814c [AhyoungRyu] Remove cancelCredentialInfoUpdate()
899bb15 [AhyoungRyu] Fix a bug reported by @Leemoonsoo
57cb280 [AhyoungRyu] Make focusing to text inputbox after update cancel
cea8c93 [AhyoungRyu] Fix typos in datasource_authorization.md
cc72ae8 [AhyoungRyu] update xeditable license version
c100a64 [AhyoungRyu] Delete interpreter_authorization.md
304e684 [AhyoungRyu] Add datasource_authorization.md docs
5768604 [AhyoungRyu] Add datasource_authorization.md to index & navi menu
64bf6fe [AhyoungRyu] Update angular-xeditable version
573c3d1 [AhyoungRyu] Enable credential info to get list, edit and remove via UI
asfgit pushed a commit that referenced this pull request Jul 18, 2016
### What is this PR for?
This PR blocks the blank values insertion on the Credential page and
changes the success message box to zeppelin's dialog box.

### What type of PR is it?
Bug Fix

### How should this be tested?
Try to save with blank values on the Credential page.

### Screenshots (if appropriate)
  - before
![out](https://cloud.githubusercontent.com/assets/3348133/16255783/c5d19378-3887-11e6-9eec-5fcac42ee276.gif)

  - after
![image](https://cloud.githubusercontent.com/assets/3348133/16255706/18967912-3887-11e6-8823-21683c17082d.png)

![image](https://cloud.githubusercontent.com/assets/3348133/16255722/55c9f494-3887-11e6-84f8-857a7f5380d6.png)

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: astroshim <hsshim@nflabs.com>

Closes #1064 from astroshim/feat/checkCredentialValues and squashes the following commits:

63bc385 [astroshim] allow blank password.
e148d44 [astroshim] disallow the blank values of Credential on the front.

(cherry picked from commit 3a338e0)
Signed-off-by: Mina Lee <minalee@apache.org>
PhilippGrulich pushed a commit to SWC-SENSE/zeppelin that referenced this pull request Aug 8, 2016
### What is this PR for?
Currently, users can add new their credential info for data source authentication in Zeppelin "Credentials" menu. Even though it was saved successfully, they can't see the whole list of credentials in Zeppelin UI.
This PR enables to `get` all credential list, `edit` and `remove` via UI.

*NOTE : Since this patch was implemented based on apache#1030 API, should be tested after apache#1030 merged.*

### What type of PR is it?
Improvement & Documentation

### Todos
* [x] - rename `interpreter_authorization.md` -> `datasource_authorization.md`
* [x] - remove `Interpreter Authorization` section (since we don't have this feature yet : [ZEPPELIN-945](https://issues.apache.org/jira/browse/ZEPPELIN-945))
* [x] - rebase after apache#1030 & apache#1064 merged
* [ ] - address reviews

### What is the Jira issue?
[ZEPPELIN-1054](https://issues.apache.org/jira/browse/ZEPPELIN-1054)

### How should this be tested?
1. Apply this patch and build `zeppelin-web` as described in [here](https://github.com/apache/zeppelin/tree/master/zeppelin-web#configured-environment).
2. Go to `Credentials` menu.
3. Add new credentials -> you can see the credential info in the credential list table.
4. You can edit & delete them. -> Compare with `conf/credentials.json`

### Screenshots (if appropriate)
- Before
<img width="952" alt="screen shot 2016-06-28 at 12 37 10 am" src="https://cloud.githubusercontent.com/assets/10060731/16407604/69b0c4d8-3cc9-11e6-8284-9abe2969cdc1.png">

- After
![add_credential](https://cloud.githubusercontent.com/assets/10060731/16576765/3671aa16-42cc-11e6-9d9f-dfe1f33f8d37.gif)
If there is no credential
<img width="957" alt="screen shot 2016-06-28 at 12 19 46 am" src="https://cloud.githubusercontent.com/assets/10060731/16407620/7838995e-3cc9-11e6-90ba-1bd0173a1b49.png">

- `datasource_authorization.md`
<img width="845" alt="screen shot 2016-06-28 at 7 58 24 pm" src="https://cloud.githubusercontent.com/assets/10060731/16439169/d4026034-3d6a-11e6-930f-86de12e5fc49.png">
<img width="851" alt="screen shot 2016-06-28 at 7 58 44 pm" src="https://cloud.githubusercontent.com/assets/10060731/16439170/d62f2842-3d6a-11e6-9d3f-ecc5cda29c77.png">
<img width="846" alt="screen shot 2016-06-28 at 8 00 20 pm" src="https://cloud.githubusercontent.com/assets/10060731/16439200/fed58390-3d6a-11e6-9aa2-8cff5a1b7b66.png">

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? yes

Author: AhyoungRyu <fbdkdud93@hanmail.net>

Closes apache#1100 from AhyoungRyu/ZEPPELIN-1054 and squashes the following commits:

7c38c90 [AhyoungRyu] Fix checkstyle error with jscs rule
ab9814c [AhyoungRyu] Remove cancelCredentialInfoUpdate()
899bb15 [AhyoungRyu] Fix a bug reported by @Leemoonsoo
57cb280 [AhyoungRyu] Make focusing to text inputbox after update cancel
cea8c93 [AhyoungRyu] Fix typos in datasource_authorization.md
cc72ae8 [AhyoungRyu] update xeditable license version
c100a64 [AhyoungRyu] Delete interpreter_authorization.md
304e684 [AhyoungRyu] Add datasource_authorization.md docs
5768604 [AhyoungRyu] Add datasource_authorization.md to index & navi menu
64bf6fe [AhyoungRyu] Update angular-xeditable version
573c3d1 [AhyoungRyu] Enable credential info to get list, edit and remove via UI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants