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

Add support for Portals Web API #156

Merged
merged 3 commits into from
Jul 24, 2023

Conversation

03-CiprianoG
Copy link
Contributor

Hey Aleksandr,
Firstly I wanted to say thank you, I really appreciate your work :)

This minor patch aims to add support to the portals Web API.
Since I'm myself using a patched version of this library, I thought I could add those few lines of code and make a minor version out of it..

As far as I know the Portals Web API uses another URL but the same protocol, so we shouldn't need additional test/types for this "feature".

Let me know what you think and keep the amazing work up :)

@03-CiprianoG 03-CiprianoG marked this pull request as draft July 21, 2023 23:56
@03-CiprianoG 03-CiprianoG marked this pull request as ready for review July 21, 2023 23:57
@AleksandrRogov
Copy link
Owner

Hi @03-CiprianoG !
This is neat! I did not realize that it was so simple to support the Portals in the library! I appreciate your time for changing the code and creating this PR.

A question for you, because I haven't worked with Portals much and don't know this information: is it possible to make it work without introducing a new config property? I was thinking that maybe if Portals have some kind of global JavaScript object that could be checked and if it exists instantly realize that the code is running there. Like, something similar to Xrm.Utility and GetGlobalContext in Dynamics 365?

If yes, we could just add a code to check for serverUrl in the configuration and if it's not set, check for that object in the Portals and if it's found then construct the url for the Portals, otherwise default to Dynamics.

What do you think?

P.S. Thank you for the kind words! 😃

@AleksandrRogov AleksandrRogov added future enhancement v2 The issue is a task for v2 development labels Jul 22, 2023
@03-CiprianoG
Copy link
Contributor Author

Hi @AleksandrRogov 🫡.
I''ve made the adjustments according to your suggestion, and now this new feature works seamlessly with the rest of the library 🥳.

The global object you kind of mentioned definitely exists and is shell, so checking for it will determine if we are running within a Portals Environment or not.
I've just added a new utility function as you said.

I'm happy with the result and I'm currently using it rn :),
Let me know what you think now and If you can come up with any other suggestion/improvement please do.

Thank you for your time again.

@AleksandrRogov
Copy link
Owner

Awesome! Well done!

I've also checked Portals documentation and found out that a token is needed to access the api. Like in this example:

    //Web API ajax wrapper
    (function(webapi, $) {
      function safeAjax(ajaxOptions) {
        var deferredAjax = $.Deferred();
        shell.getTokenDeferred().done(function(token) {
          // Add headers for ajax
          if (!ajaxOptions.headers) {
            $.extend(ajaxOptions, {
              headers: {
                "__RequestVerificationToken": token
              }
            });
          } else {
            ajaxOptions.headers["__RequestVerificationToken"] = token;
          }
          $.ajax(ajaxOptions)
            .done(function(data, textStatus, jqXHR) {
              validateLoginSession(data, textStatus, jqXHR, deferredAjax.resolve);
            }).fail(deferredAjax.reject); //ajax
        }).fail(function() {
          deferredAjax.rejectWith(this, arguments); // On token failure pass the token ajax and args
        });
        return deferredAjax.promise();
      }
      webapi.safeAjax = safeAjax;
    })(window.webapi = window.webapi || {}, jQuery)

Do you have an example on how to do that with DynamicsWebApi so I could add it to the documentation? Thanks!

I will merge this later (also add some tests) and I also wanted to add another feature, so I will release both of them at the same time.

Thanks again for your help!

Copy link
Owner

@AleksandrRogov AleksandrRogov left a comment

Choose a reason for hiding this comment

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

  1. Need to make sure that the node.js app does not break because of the window object.
  2. Maybe move the conditions around so the parts that aren't used in Node.js can be removed by tree shaking.
  3. Make sure the bundling process is working fine.
  4. Add tests.

@AleksandrRogov AleksandrRogov merged commit 5737313 into AleksandrRogov:master Jul 24, 2023
AleksandrRogov added a commit that referenced this pull request Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future enhancement v2 The issue is a task for v2 development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants