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

feat: Provide web gui for {list, add, add, update, remove} repository… #106

Merged
merged 2 commits into from
Oct 24, 2022

Conversation

andpopov
Copy link
Member

Closes: #105
@olenagerasimova, @dgarus Please review

Copy link
Member

@olenagerasimova olenagerasimova left a comment

Choose a reason for hiding this comment

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

@andpopov thanks, check my comment and

  1. the main idea to remake front was to avoid it to depend on the same config as Artipie, so front should not use artipie.yaml as settings, Artipie API url should be eventually the only setting required for front to work.
  2. I think API endpoint should be removed, so they do not complicate current code and do not lead to any not required changes in the code
  3. design issues: will we work on design later? Now, at least, working (I've colored it with blue) area is not in the middle of the page...
    image

There is no need to fix these three comments in this PR, but they can definitely should be addressed later.

* @param <V> Type of resulting of map-function.
* @return Triple-tuple.
*/
protected static <V> Tuple3<Integer, V, String> handle(final int success,
Copy link
Member

Choose a reason for hiding this comment

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

There tuples with several items does not look clear and it's not convenient to use them: you should always know what item represents and perform various checks...
Spark has error handling and exception mappers, maybe we can throw some custom exception on error from Artipie API and handle it with spark? Thus no tuples with nulls and no checks will be required...

Copy link
Member

@olenagerasimova olenagerasimova left a comment

Choose a reason for hiding this comment

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

@andpopov approving as we discussed

@andpopov andpopov merged commit f1e8106 into artipie:101_dashboard Oct 24, 2022
@andpopov andpopov deleted the 105_repository_web_gui branch October 26, 2022 15:55
andpopov added a commit that referenced this pull request Oct 28, 2022
* feat: Singn in and obtaining JWT-token (#103)

* feat: Singn in and obtaining JWT-token

* feat: Provide web gui for {list, add, add, update, remove} repository… (#106)

* feat: Provide web gui for {list, add, add, update, remove} repository settings

* add standard artipie authentication

* Start web gui without artipie yaml configuration (#107)

feat: start dashboard without config

* Remove rest services from frontend (#108)

feat: remove rest services from front
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