Load Template from Gitlab Functionality Added by RounakJoshi09 · Pull Request #14 · cophilot/templify

RounakJoshi09 · 2024-09-10T19:50:51Z

Added the feature to load the template from GitLab repository.

Type of change

Please select the relevant option:

Description

To imlement this functionality, I researched that the url needed to fetch files from gitlab is little different than github, also gitlab have one api to give the folder/dir of the template and another api to give the content of the blob/file.
The content of the file which we recieve is encoded in base64 format, which needs to be decoded in order to get the content of the file.
Also to make the things resuable , I have added a parameter, named urrlType to the existing function and based on this I am driving the code to the specific function fot gitlab and github.

Changes

Things I did for this feature:-

Added a package to decode base64 string, because gitlab gives the content of file in base64.
Added a new method to load template/dir from gitlab
Added tests to make sure everything works as expected.
Added required changes in the help description.

RounakJoshi09 · 2024-10-13T08:29:27Z

Hi @cophilot , I have completed all the changes along with necessary tests for this PR. Please check and let me know if you want any changes. Also as soon as you read this, please assign me with some other task / bug to contribute on.

cophilot · 2024-10-28T18:17:00Z

src/commands/load.rs

+
+    let url_type = if url.starts_with("https://github.com") {
+        URLType::GitHub
+    } else if url.starts_with("https://gitlab.com") {


Also costum giotlab isntrances should be supported. E.g. then you can have an URL like "https://my.gitlab.com". I think here a a pattern check would be better (like "https://*gitlab.com")

Hi @cophilot , Thankyou so much for pointing this out , now I'm checking the gitlab url with RegEx, for a thorough pattern check

cophilot · 2024-10-28T18:22:09Z

src/utils/template_handler.rs

+    path: &str,
+    url: &str,
+    force: bool,
+    url_type: &URLType,


Each time this function is called the URL has to be checked to determine the right type. This adds code repetition. Therefore I would suggest to handle the URL type within this function and also check for if the URL is not supported. In this case return a status with an error and the caller then has to check that status as well.

Thats an amazing point @cophilot , added the url check inside this function, so that this function can become more resuable with lesser code repition. Also, added the check inside for an invalid url and added the test for the same.

cophilot · 2024-10-28T18:22:48Z

src/utils/template_handler.rs

    let response: serde_json::Value = response.unwrap();

-    let items = response["payload"]["tree"]["items"].as_array().unwrap();
+    let items = match url_type {


Thankyou 😊

cophilot · 2024-10-28T18:28:12Z

src/utils/template_handler.rs

-        ));
-    }
+/// Load a template from a gitlab repository
+pub(crate) fn load_gitlab_template(


This should not be public because the function is only used here

Got it, Made the change

cophilot · 2024-10-28T18:28:53Z

src/utils/template_handler.rs

+}
+
+/// Load a template from a github repository
+pub(crate) fn load_github_template(


Also not public

cophilot · 2024-10-28T18:33:14Z

src/utils/template_handler.rs

+}
+
+/// Load a template from a remote repository
+pub(crate) fn load_remote_template(


Move that function above load_gitlab_template and load_github_templatebecause this function should be used and therefore should be seen first

For sure, changed the position of function, for better readablity and maintainablity as you suggested

cophilot · 2024-10-28T18:34:03Z

src/utils/template_handler.rs

+    path: &str,
+    url: &str,
+    force: bool,
+    url_type: &URLType,


Same here with the type. Maybe add a internal function that is checking the type to avoid repetition of the check

cophilot · 2024-10-28T18:37:18Z

src/utils/template_handler.rs

+    for item in items {
+        if item["type"] == "tree" {
+            let st = load_remote_gitlab_template_dir(
+                format!("{}/{}", path, item["name"])


Scince this formatting is done here twice maybe add a lambda just before and then u just have to apply the lambda to the path and the url.

Sure @cophilot , Instead of Lambda function, I extracted a whole new function for this fomatting purpose, and put those at the last of template_handler.rs only. As it is using in many other places other than this, as pointed by you.

cophilot · 2024-10-28T18:38:09Z

src/utils/template_handler.rs

+    let response = rest::json_call(url);
+    if response.is_err() {
+        return Status::error(format!(
+            "Failed to get template from {}: : Request failed",


Remove one colon :)

cophilot · 2024-10-28T18:39:19Z

src/utils/template_handler.rs

+    for item in items {
+        if item["type"] == "tree" {
+            let st = load_remote_gitlab_template_dir(
+                format!("{}/{}", path, item["name"])


Same here with the lambdas. Maybe then it make sense to add a function for this...

cophilot · 2024-10-28T18:39:43Z

src/utils/template_handler.rs

+        }
+
+        load_remote_gitlab_template_file(
+            format!("{}/{}", path, item["name"])


Yeah add a small internal function for it :D

cophilot · 2024-10-28T18:49:00Z

tests/command_tests/load_test.rs

+pub fn check_gitlab_load() {
+    utils::init_tpy();
+
+    utils::run_successfully("tpy load https://gitlab.com/api/v4/projects/cophilot%2Ftemplify-vault/repository/tree?path=Test");


I see that only direct api calls are supported. Would it be possible to do it as well with the "normal url" of gitlab (e.g. "https://gitlab.com/cophilot/templify-vault/-/tree/main/Test")? Maybe parse the "normal url" to the api url? TBH not sure how easy that is...

Thats a wonderful idea, @cophilot and I am more than happy to implement this no matter how easy or difficult it will be, as it will give more flexibility to the user while loading templates from gitlab. But I am thinking to take this as an enhancement in a separate PR. Will that be fine with you ? If yes,, Please rasie a separte issue of that , and assign it to me.

cophilot · 2024-10-28T19:09:55Z

Hi @RounakJoshi09 , sorry for the late reply, I was pretty busy...
Very nice work you did here! I added some comments with small code changes but I think they are quiet easy to fix :)
Also your changes are not passing the comment-check and the lint check in the CI pipeline. Please also fix the issues that occure when running the scripts (are also quiet easy to fix). You can find more details here.

RounakJoshi09 · 2024-11-01T16:52:49Z

src/utils/template_handler.rs

+    path: &str,
+    url: &str,
+    force: bool,
+    url_type: Option<&URLType>,


I kept url_type as optional here because at some places this function is called where url_type is already calculated, but in some url_type is not there, so in order to provide more flexiibilty, I made this as optional, so if url_type is calculated before and is passed into this function we will take thaaat otherwise will recalculate later in this code at line 177,

RounakJoshi09 · 2024-11-01T17:00:02Z

I greatly appreciate your patience and thorough review of this PR, @cophilot . I'll certainly keep these principles of code reusability and maintainability in mind for future contributions. I've implemented all the changes you suggested. Could you please take another look at the updated code? For your reference, I've also attached the latest test result screenshot. Thank you again for your guidance and support.

cophilot · 2024-11-02T17:13:00Z

src/commands/load.rs


+/// This enum is used to define type of URL (.i.e.. GitHub, GitLab)
+#[derive(Clone)]
+pub(crate) enum URLType {


Please move that Type to a own file under src/types.
Otherwise the PR looks good and can be merged after that small change :)

Made the change here @cophilot 94b7250

Thankyou

RounakJoshi09 · 2024-11-03T06:55:16Z

cophilot · 2024-11-03T09:02:39Z

@RounakJoshi09 You don`t have to run the test manually and show them, the pipeline is triggered automatically :)

cophilot · 2024-11-03T09:03:09Z

Well done! Will merge now

RounakJoshi09 · 2024-11-03T10:53:20Z

Thanks for merging this @cophilot , At last I would request to change readme.md of this gitlab repository
https://gitlab.com/cophilot/templify-vault , and update all the load urls of the vault to the gitlab urls, for users convenience.

cophilot · 2024-11-03T10:55:01Z

Thanks for merging this @cophilot , At last I would request to change readme.md of this gitlab repository

https://gitlab.com/cophilot/templify-vault , and update all the load urls of the vault to the gitlab urls, for users convenience.

Good Idea @RounakJoshi09 !
I will do that

RounakJoshi09 added 3 commits September 9, 2024 10:04

Github Method Extracted and Enum Declared for URL Type - 1.0.0

b7360ed

Added Load File From Gitlab - 1.0.0 - 1.0.0

1a7b014

Load Collection Function Updated for Gitlab - 1.0.0 - 1.0.0

8c222c8

RounakJoshi09 mentioned this pull request Sep 10, 2024

Load from Gitlab #11

Closed

Code Refactoring - 1.0.0

a4c9298

cophilot marked this pull request as ready for review September 22, 2024 06:49

Test Created for Gitlab Feature - 1.0.0

3dff8b0

cophilot linked an issue Oct 28, 2024 that may be closed by this pull request

Load from Gitlab #11

Closed

cophilot reviewed Oct 28, 2024

View reviewed changes

cophilot assigned RounakJoshi09 Oct 28, 2024

Code Refactored - 1.0.0

a5e3e05

RounakJoshi09 commented Nov 1, 2024

View reviewed changes

Linting and Comment Check done - 1.0.0 - 1.0.0

19d19b2

RounakJoshi09 force-pushed the rounakj/gitlab branch from a713c2e to 19d19b2 Compare November 1, 2024 16:58

cophilot changed the base branch from master to dev November 2, 2024 16:49

cophilot mentioned this pull request Nov 2, 2024

Gitlab URL Regex support #17

Open

cophilot reviewed Nov 2, 2024

View reviewed changes

url Type Extracted - 1.0.0

94b7250

cophilot merged commit c0a3b2b into cophilot:dev Nov 3, 2024

Conversation

RounakJoshi09 commented Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Description

Changes

Uh oh!

RounakJoshi09 commented Oct 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RounakJoshi09 Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cophilot Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RounakJoshi09 Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cophilot commented Oct 28, 2024

Uh oh!

RounakJoshi09 Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RounakJoshi09 commented Nov 1, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RounakJoshi09 Nov 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RounakJoshi09 commented Nov 3, 2024

Uh oh!

cophilot commented Nov 3, 2024

Uh oh!

cophilot commented Nov 3, 2024

Uh oh!

RounakJoshi09 commented Nov 3, 2024

Uh oh!

cophilot commented Nov 3, 2024

Uh oh!

Reviewers

Assignees

Labels

RounakJoshi09 commented Sep 10, 2024 •

edited

Loading

RounakJoshi09 Nov 1, 2024 •

edited

Loading

cophilot Oct 28, 2024 •

edited

Loading

RounakJoshi09 Nov 1, 2024 •

edited

Loading

RounakJoshi09 Nov 1, 2024 •

edited

Loading

RounakJoshi09 Nov 3, 2024 •

edited

Loading