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

Gym management app #114

Merged
merged 64 commits into from
Nov 19, 2024
Merged

Conversation

Marvelous-O
Copy link
Contributor

  • Bug fix?
  • New sample?
  • Related issues: fixes #X, partially #Y, mentioned in #Z
  • Needs API permissions?
  • Has other prerequisites? (E.g. requires a list, document library, etc.)

What's in this Pull Request?

 New Sample

Checklist

  • My pull request affects only ONE sample.
  • I have updated the README file.
  • My README has at least one static high-resolution screenshot (i.e. not a GIF)
  • My README contains complete setup instructions, including pre-requisites and permissions required

@Katerina-Chernevskaya
Copy link
Contributor

Hi @Marvelous-O ,

Thank you for your contribution! With the source code in this PR I'm able to compile the solution and import it. However, to proceed I would suggest several changes:

  1. Please check if you are using the image "kisspng-check-mark-portable-network-graphics-computer-icon-semplifisco-fiscalit-locale-5c1129251c4338.4414213515446285171158.png" in your app. If so, please rename the image to keep shorter name. This long name affect the clone process and it won't be suitable for users to resolve this issue by themself.

  2. Please use connection references for flows and include these connection references in your solution to avoid manual edit steps for flows. Please find more related information here. By using connection references users will specify the SharePoint site and list during the import and after the import no manual steps would be required.

  3. Please check that all fields required for flows are mentioned in the instructions. I stuck with several fields in the Copy Due Date flow that are not in the SharePoint list:
    image

Please update your sample and we will proceed with review and merge steps.

Regards,
Katerina Ch.

@Katerina-Chernevskaya Katerina-Chernevskaya added the Needs Attention Something needs to be fixed with the PR before merging label Oct 31, 2024
@Marvelous-O
Copy link
Contributor Author

Thanks a lot @Katerina-Chernevskaya for your help and instructions. Adjusted, kindly verify. Thank you

@Katerina-Chernevskaya
Copy link
Contributor

Hi @Marvelous-O ,

Thank you for updates! However, there are some new errors appear:

  1. I notice that you are using SharePoint list in the Power Apps Canvas app as well. And you hardcoded your SharePoint list. Hardcoded value should be replaces with the Environment variable. You already have Environment variables for site and list and successfully using them in flows. Great! Now let's replace hardcoded value in the app.
    1 - search for the SharePoint datasource
    2 - select it
    3 - navigate to Advanced
    4 - select you environment variable for the site EnvVariables
    5 - navigate to Advanced for the list selection
    6 - select your environment variable for the list ListVariables
    7 - click Connect
    image
    image
    image

Once you added it, you can remove the hardcoded value from the Data source tab. Also you will need to replace the hardcoded value in all controls.
For gallery you can just replace the value:
image

For fields in the forms I'm afraid only way is to remove old fields and add fields again after you update the form's source value.

  1. Fields in the Membership Notification flow seems to be not the same as in the readme file:
    The field To refer to the field E-mail, however, in the readme guide you ask to create field with the title E-mail Address :
    image

Could you please check all flows to remove such issues. Because otherwise users who will follow you guide won't be able to use the flows that refer to a different fields name.

Once you adjust your solution, I recommend you to test it in a new environment - import the solution and check that everything is working as expected.

I'm waiting for your new PR and will proceed. If you have any questions or need support - please let me know!

Rerads,
Katerina Ch.

@Marvelous-O
Copy link
Contributor Author

Hi, @Katerina-Chernevskaya so glad I was able to find the bug, found out the SharePoint connection was referencing the internal name of the columns in the SharePoint list due to how my SharePoint list column was named, there were spaces in my SharePoint column names e.g. Due Date, SharePoint referenced it as Duex0200Date, so while importing it to a new environment, the internal reference name differs from the Test Environment, SharePoint auto generates those numbers in the middle to cover up those spaces, which differs from each environment. I guess naming a SharePoint List column like this: "Due Date" isn't a good approach.

@Katerina-Chernevskaya Katerina-Chernevskaya removed the Needs Attention Something needs to be fixed with the PR before merging label Nov 19, 2024
Copy link
Contributor

@Katerina-Chernevskaya Katerina-Chernevskaya left a comment

Choose a reason for hiding this comment

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

Hi @Marvelous-O ,

Thank you for sharing this cool sample! I've made some changes in the ReadMe file to align it with the template, and added the sample.json file. I'm going to merge your sample.

Thank you for your contribution!

Katerina Ch.

@Katerina-Chernevskaya Katerina-Chernevskaya merged commit 9c545ad into pnp:main Nov 19, 2024
1 check passed
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